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

Fix escaping for password for alembic migrations #24125

Closed
wants to merge 1 commit into from
Closed

Fix escaping for password for alembic migrations #24125

wants to merge 1 commit into from

Conversation

pdbaines
Copy link

@pdbaines pdbaines commented May 18, 2023

There is a bug in the current superset tip where passwords with a % are not handled correctly in the alembic migration script (they are handled appropriately elsewhere).

This recent PR: #23176 was designed to handle URLs with a @ symbol, but does not work for passwords with% signs. The root cause is clearly documented here, in that set_main_option requires escaping of % characters.

Just setting the escaped password as the secret value does not work, as that value is used correctly elsewhere for db connections (with correct escape character handling), so in those instances you get password errors due to the extra character.

@pdbaines pdbaines requested a review from a team as a code owner May 18, 2023 22:39
@pdbaines pdbaines closed this May 18, 2023
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️

We hope to see you in our Slack community too! Not signed up? Use our Slack App to self-register.

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

Successfully merging this pull request may close these issues.

1 participant