potiuk commented on a change in pull request #17423:
URL: https://github.com/apache/airflow/pull/17423#discussion_r683000067



##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -70,6 +70,7 @@ def __init__(self, *args, **kwargs) -> None:
         super().__init__(*args, **kwargs)
         self.connection: Optional[Connection] = kwargs.pop("connection", None)
         self.conn: connection = None
+        self.schema: Optional[str] = kwargs.pop("schema", None)

Review comment:
       I think this is unlikely edge case (much less likely than use of schema 
when it will be defined in DBApiHook). I hardly see the use for that. Why 
somone would like to define a DBApi -derived hook and pass a 'schema' parameter 
to it while also changing it in it's own `__init__`? Seems extremely unlikely, 
also It feels natural that in such case the change should be done before 
`super.__init__()` rather than after.   
   
   We can assume that "schema" is our convention for kwarg para that we should 
use for all DB Hooks.  We can standardise it (it's already commonly used)  and 
release in Airflow 3 DBApiHook I think. 
   
   For now we might want to add some more docs/comments explaining it and some 
Airflow 3 notice about it. WDYT? 




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to