You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "jscheffl (via GitHub)" <gi...@apache.org> on 2025/07/12 19:48:37 UTC
[PR] Cleanup type ignores in vertica provider where possible [airflow]
jscheffl opened a new pull request, #53252:
URL: https://github.com/apache/airflow/pull/53252
Relates to https://github.com/apache/airflow/issues/53241
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Cleanup type ignores in vertica provider where possible [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #53252:
URL: https://github.com/apache/airflow/pull/53252#discussion_r2203237161
##########
providers/vertica/src/airflow/providers/vertica/hooks/vertica.py:
##########
@@ -67,7 +67,7 @@ class VerticaHook(DbApiHook):
def get_conn(self) -> connect:
"""Return vertica connection object."""
- conn = self.get_connection(self.vertica_conn_id) # type: ignore
+ conn = self.get_connection(self.vertica_conn_id)
Review Comment:
yeah looks like bug, we can set `default_conn_name` ?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Cleanup type ignores in vertica provider where possible [airflow]
Posted by "kyungjunleeme (via GitHub)" <gi...@apache.org>.
kyungjunleeme commented on PR #53252:
URL: https://github.com/apache/airflow/pull/53252#issuecomment-3066894273
#53311
this case same case
```
"""Establish a connection to pinot broker through pinot dbapi."""
conn = self.get_connection(self.pinot_broker_conn_id) # type: ignore
```
```python
"""Establish a connection to pinot broker through pinot dbapi."""
conn = self.get_connection(self.get_conn_id())
```
get_conn_id() is a method defined in DbApiHook that dynamically returns the appropriate connection ID by referencing the value of self.conn_name_attr.
It's important to document these things well to prevent potential bugs.
---
I noticed you had already investigated and added similar cases like this one, so I'm adding this as well. Thank you!
@jscheffl
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Cleanup type ignores in vertica provider where possible [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #53252:
URL: https://github.com/apache/airflow/pull/53252#discussion_r2203233296
##########
providers/vertica/src/airflow/providers/vertica/hooks/vertica.py:
##########
@@ -67,7 +67,7 @@ class VerticaHook(DbApiHook):
def get_conn(self) -> connect:
"""Return vertica connection object."""
- conn = self.get_connection(self.vertica_conn_id) # type: ignore
+ conn = self.get_connection(self.vertica_conn_id)
Review Comment:
yes
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Cleanup type ignores in vertica provider where possible [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan merged PR #53252:
URL: https://github.com/apache/airflow/pull/53252
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Cleanup type ignores in vertica provider where possible [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #53252:
URL: https://github.com/apache/airflow/pull/53252#discussion_r2202912638
##########
providers/vertica/src/airflow/providers/vertica/hooks/vertica.py:
##########
@@ -67,7 +67,7 @@ class VerticaHook(DbApiHook):
def get_conn(self) -> connect:
"""Return vertica connection object."""
- conn = self.get_connection(self.vertica_conn_id) # type: ignore
+ conn = self.get_connection(self.vertica_conn_id)
Review Comment:
@potiuk / @gopidesupavan I am scratching my head and taking a look to the code.... as well as to the history. The field `vertica_conn_id` actually is not existing and seems invalid - for thi sis actually broken and can not be used. `type ignore` is just masking the problem currently.
I am not sure what is "right", I assume an `__init__()` is missing which defined the field (but this would be breaking for all users to define this in constructor) or it would need to use `conn_name_attr`.
```suggestion
conn = self.get_connection(self.conn_name_attr)
```
```suggestion
conn = self.get_connection(self.vertica_conn_id)
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Cleanup type ignores in vertica provider where possible [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #53252:
URL: https://github.com/apache/airflow/pull/53252#discussion_r2202912638
##########
providers/vertica/src/airflow/providers/vertica/hooks/vertica.py:
##########
@@ -67,7 +67,7 @@ class VerticaHook(DbApiHook):
def get_conn(self) -> connect:
"""Return vertica connection object."""
- conn = self.get_connection(self.vertica_conn_id) # type: ignore
+ conn = self.get_connection(self.vertica_conn_id)
Review Comment:
@potiuk / @gopidesupavan I am scratching my head and taking a look to the code.... as well as to the history. The field `vertica_conn_id` actually is not existing and seems invalid - for thi sis actually broken and can not be used. `type ignore` is just masking the problem currently.
I am not sure what is "right", I assume an `__init__()` is missing which defined the field (but this would be breaking for all users to define this in constructor) or it would need to use `conn_name_attr`.
```suggestion
conn = self.get_connection(self.conn_name_attr)
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Cleanup type ignores in vertica provider where possible [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #53252:
URL: https://github.com/apache/airflow/pull/53252#discussion_r2203233403
##########
providers/vertica/src/airflow/providers/vertica/hooks/vertica.py:
##########
@@ -67,7 +67,7 @@ class VerticaHook(DbApiHook):
def get_conn(self) -> connect:
"""Return vertica connection object."""
- conn = self.get_connection(self.vertica_conn_id) # type: ignore
+ conn = self.get_connection(self.vertica_conn_id)
Review Comment:
conn_name_attr
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Cleanup type ignores in vertica provider where possible [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #53252:
URL: https://github.com/apache/airflow/pull/53252#discussion_r2202912638
##########
providers/vertica/src/airflow/providers/vertica/hooks/vertica.py:
##########
@@ -67,7 +67,7 @@ class VerticaHook(DbApiHook):
def get_conn(self) -> connect:
"""Return vertica connection object."""
- conn = self.get_connection(self.vertica_conn_id) # type: ignore
+ conn = self.get_connection(self.vertica_conn_id)
Review Comment:
@potiuk / @gopidesupavan I am scratching my head and taking a look to the code.... as well as to the history. The field `vertica_conn_id` actually is not existing and seems invalid - for thi sis actually broken and can not be used. `type ignore` is just masking the problem currently.
I am not sure what is "right", I assume an `__init__()` is missing which defined the field (but this would be breaking for all users to define this in constructor) or it would need to use `conn_name_attr`.
```suggestion
conn = self.get_connection(self.get_conn_id())
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Cleanup type ignores in vertica provider where possible [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #53252:
URL: https://github.com/apache/airflow/pull/53252#discussion_r2203237900
##########
providers/vertica/src/airflow/providers/vertica/hooks/vertica.py:
##########
@@ -67,7 +67,7 @@ class VerticaHook(DbApiHook):
def get_conn(self) -> connect:
"""Return vertica connection object."""
- conn = self.get_connection(self.vertica_conn_id) # type: ignore
+ conn = self.get_connection(self.vertica_conn_id)
Review Comment:
its good that mypy exposed actual bugs , like this one and other providers google and samba
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Cleanup type ignores in vertica provider where possible [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #53252:
URL: https://github.com/apache/airflow/pull/53252#discussion_r2203237535
##########
providers/vertica/src/airflow/providers/vertica/hooks/vertica.py:
##########
@@ -67,7 +67,7 @@ class VerticaHook(DbApiHook):
def get_conn(self) -> connect:
"""Return vertica connection object."""
- conn = self.get_connection(self.vertica_conn_id) # type: ignore
+ conn = self.get_connection(self.vertica_conn_id)
Review Comment:
Changing it now to `self.get_connection(self.get_conn_id())` which makes most sense.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org