BugFix: Improve support for special characters in DbApiHook.get_uri #12775
Conversation
airflow/hooks/dbapi_hook.py
Outdated
| @@ -78,7 +79,7 @@ def get_uri(self) -> str: | |||
| conn = self.get_connection(getattr(self, self.conn_name_attr)) | |||
| login = '' | |||
| if conn.login: | |||
| login = '{conn.login}:{conn.password}@'.format(conn=conn) | |||
| login = '{conn.login}:{quote_plus(conn.password)}@'.format(conn=conn) | |||
There was a problem hiding this comment.
Does this problem also apply to the login?
There was a problem hiding this comment.
Good point. It could. To be safe, we should apply the same to login too.
There was a problem hiding this comment.
It does -- hosted Postgresql service on Azure has an @ in the username.
alokshenoy
force-pushed
the
fix-urlencoded-db-password
branch
from
4c06047 to
668f1b6
Compare
|
The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*. |
|
The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*. |
|
Looking into the static check failure. |
|
The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*. |
alokshenoy
force-pushed
the
fix-urlencoded-db-password
branch
from
cee827c to
643143f
Compare
Closes [apache#12722](apache#12722) raised by @NadimYounes - Uses urllib.parse.quote_plus to urlencode the username (login) and password since SQLAlchemy requires a valid URI to create an engine. - Adds test case that checks for encoding of the username (login) and password. Co-authored-by: Ash Berlin-Taylor <[email protected]>
alokshenoy
force-pushed
the
fix-urlencoded-db-password
branch
from
643143f to
6127e8a
Compare
|
@NadimYounes , @ashb - This is ready to be reviewed. |
|
@mik-laj - does it look good for you as well ? I would like to merge it, but need a second pair of eyes :) |
|
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
github-actions
bot
added
the
full tests needed
mik-laj
changed the title
Closes #12722 raised by @NadimYounes
Uses urllib.parse.quote_plus to urlencode the password since SQLAlchemy requires a valid URI to create an engine.
Adds test case that checks for encoding of password.