uranusjr commented on a change in pull request #16521:
URL: https://github.com/apache/airflow/pull/16521#discussion_r654844308
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -115,6 +116,23 @@ def get_conn(self) -> connection:
self.conn = psycopg2.connect(**conn_args)
return self.conn
+ def get_uri(self) -> str:
+ """
+ Extract the URI from the connection.
+
+ :return: the extracted uri.
+ """
+ conn = self.get_connection(getattr(self, self.conn_name_attr))
+ login = ''
+ if conn.login:
+ login = f'{quote_plus(conn.login)}:{quote_plus(conn.password)}@'
+ host = conn.host
+ if conn.port is not None:
+ host += f':{conn.port}'
+ schema = self.schema or conn.schema or ''
+ uri = urlunsplit((conn.conn_type, f'{login}{host}', schema, '', ''))
+ return uri
Review comment:
I wonder if we should do some refactoring to eliminate the duplicate
code between this and `DbApiHook.get_uri()`, they are way too similar
(basically only one different line). Maybe we should promote `self.schema` and
this implementation to `DbApiHook`, and setting `schema` on `PostgresHook` will
automatically get the correct behaviour. This also eliminates the possible bug
if another database also has a schema concept but we forget to override
`get_uri()` for it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
