Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename ToolsDB data source #27

Merged
merged 1 commit into from
Aug 22, 2024
Merged

Rename ToolsDB data source #27

merged 1 commit into from
Aug 22, 2024

Conversation

dhinus
Copy link
Member

@dhinus dhinus commented Aug 1, 2024

The current database name is wrong, the real database name is specified in the connection URL and is "information_schema".

As discussed in the Phab task, it makes sense to call this data source simply "ToolsDB" and let users select their desired database in their SQL queries.

Bug: T367393

@dhinus dhinus requested a review from vivian-rook August 1, 2024 14:23
@dhinus
Copy link
Member Author

dhinus commented Aug 2, 2024

@vivian-rook thanks for approving! Will you take care of merging & deploying this, or should I do it? This is not urgent by the way, it can wait a few days.

@dhinus
Copy link
Member Author

dhinus commented Aug 22, 2024

@vivian-rook I see that T372395 is now resolved, can I try deploying this PR with deploy.sh?

@vivian-rook
Copy link
Contributor

@vivian-rook I see that T372395 is now resolved, can I try deploying this PR with deploy.sh?

Go for it!

The current database name is wrong, the real database name is specified
in the connection URL and is "information_schema".

As discussed in the Phab task, it makes sense to call this data source
simply "ToolsDB" and let users select their desired database in their
SQL queries.

Bug: T367393
@dhinus
Copy link
Member Author

dhinus commented Aug 22, 2024

I deployed this PR, I can see the new name, but the old one is also there:

Screenshot 2024-08-22 at 15 45 45

@vivian-rook
Copy link
Contributor

Might be hiding somewhere in the database. We could deploy a fresh system, that would probably remove the old entry.

@dhinus
Copy link
Member Author

dhinus commented Aug 22, 2024

kubectl get configmap superset-extra-config -o yaml shows that the old name is no longer present in the k8s config.

@vivian-rook do you know why the old name is still showing in the UI, next to the new one?

@dhinus
Copy link
Member Author

dhinus commented Aug 22, 2024

Sorry, I posted my comment before reading yours :)

@dhinus
Copy link
Member Author

dhinus commented Aug 22, 2024

What's the procedure to deploy a fresh system? tofu destroy?

@vivian-rook
Copy link
Contributor

For how it would be redeployed, it is poorly described in the README. It follows the same procedure in https://wikitech.wikimedia.org/wiki/PAWS/Admin#Blue_Green_Deployment then the database migrate steps described in the README

@dhinus
Copy link
Member Author

dhinus commented Aug 22, 2024

Thanks! The UI issue is now fixed (did it require a full redeploy?), so I think this can be merged.

@dhinus dhinus merged commit 548f178 into main Aug 22, 2024
1 check passed
@dhinus dhinus deleted the rename_toolsdb branch August 22, 2024 16:15
@vivian-rook
Copy link
Contributor

Thanks! The UI issue is now fixed (did it require a full redeploy?), so I think this can be merged.

No a redeploy was not required. The database only had one dataset so the UI allowed me to remove it after removing the dataset (For some reason the UI won't allow you to remove objects that are integrated. Can get very problematic sometimes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants