xinbinhuang commented on a change in pull request #16521:
URL: https://github.com/apache/airflow/pull/16521#discussion_r655743068
##########
File path: airflow/hooks/dbapi.py
##########
@@ -53,6 +53,8 @@ class DbApiHook(BaseHook):
supports_autocommit = False
# Override with the object that exposes the connect method
connector = None # type: Optional[ConnectorProtocol]
+ # Override to use different schema from connection setting
+ schema = None
Review comment:
> self.schema = None in its __init__ without worrying the value getting
overridden by super().__init__.
Hmm, shouldn't `super().__init__` being ran before other subclass-specific
params? Also, all other superclass shouldn't worry about `self.schema =
<schema>` because it should be captured by `super().__init__()`
```python
def __init__(self, *args, **kwargs):
super.__init__(*args, **kwargs) # `schema` should go in here
```
I think it's rather confusing right now because we're mixing class attribute
and instance attribute. What's the purpose of `schema` at the class level? When
we do `self.schema = None` in subclass, the class attribture `schema` will
simply get ignored and hidden by the instance attibute `self.schema`. i.e.
```python
class Parent:
schema = None
class Child(Parent):
def __init__(self, schema):
self.schema = schema
child = Child('postgres')
print(child.__class__.schema)
print(child.schema)
print(Child.schema)
print(Parent.schema)
# None
# postgres
# None
# None
```
--
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]
