You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "bourliam (via GitHub)" <gi...@apache.org> on 2024/06/11 12:32:37 UTC
[I] Google provider : Unnecessary imports for CloudSQL operators [airflow]
bourliam opened a new issue, #40175:
URL: https://github.com/apache/airflow/issues/40175
### Apache Airflow Provider(s)
google
### Versions of Apache Airflow Providers
google, ssh, slack
### Apache Airflow version
2.6.3
### Operating System
composer
### Deployment
Google Cloud Composer
### Deployment details
_No response_
### What happened
When trying to load the dag for a test with a CloudSQLExportInstanceOperator (from airflow.providers.google.cloud.operators.cloud_sql) I have a module not found error (ModuleNotFoundError: No module named 'airflow.providers.mysql').
I do not use mysql, so I should not have to install this module.
### What you think should happen instead
We should not need those dependencies if not necessary. Moreover it is only used for type checking in another operator (CloudSQLExecuteQueryOperator)
### How to reproduce
Import a CloudSQL operator without the mysql provider installed for example
### Anything else
_No response_
### Are you willing to submit PR?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Google provider : Unnecessary imports for CloudSQL operators [airflow]
Posted by "bourliam (via GitHub)" <gi...@apache.org>.
bourliam commented on issue #40175:
URL: https://github.com/apache/airflow/issues/40175#issuecomment-2162440162
Hello @shahar1, Thanks for the reply but this was not solved (at least not for me) :
I have been digging a little bit more and the module not found error comes from the file google/cloud/hooks/cloud_sql.py where the mysql import is "mandatory" even if I do not use MySql databases.
--
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: [I] Google provider : Unnecessary imports for CloudSQL operators [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on issue #40175:
URL: https://github.com/apache/airflow/issues/40175#issuecomment-2248472161
> Hello. I am interested in working on this issue. Can I take it over?
I assigned you :)
--
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: [I] Google provider : Unnecessary imports for CloudSQL operators [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 closed issue #40175: Google provider : Unnecessary imports for CloudSQL operators
URL: https://github.com/apache/airflow/issues/40175
--
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: [I] Google provider : Unnecessary imports for CloudSQL operators [airflow]
Posted by "jhongy1994 (via GitHub)" <gi...@apache.org>.
jhongy1994 commented on issue #40175:
URL: https://github.com/apache/airflow/issues/40175#issuecomment-2248460332
Hello. I am interested in working on this issue. Can I take it over?
--
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: [I] Google provider : Unnecessary imports for CloudSQL operators [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 closed issue #40175: Google provider : Unnecessary imports for CloudSQL operators
URL: https://github.com/apache/airflow/issues/40175
--
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: [I] Google provider : Unnecessary imports for CloudSQL operators [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on issue #40175:
URL: https://github.com/apache/airflow/issues/40175#issuecomment-2162469454
> airflow.providers.google.cloud.operators.cloud_sql
Oh, I think that I see it now - I assume that you're referring to these imports in `hooks/cloud_sql.py`:
```python
from airflow.providers.mysql.hooks.mysql import MySqlHook
from airflow.providers.postgres.hooks.postgres import PostgresHook
```
They are currently being used in this operator for initializing the hook, not only for typing:
```python
def get_database_hook(self, connection: Connection) -> PostgresHook | MySqlHook:
"""Retrieve database hook.
This is the actual Postgres or MySQL database hook that uses proxy or
connects directly to the Google Cloud SQL database.
"""
if self.database_type == "postgres":
db_hook: PostgresHook | MySqlHook = PostgresHook(connection=connection, schema=self.database) # <-
else:
db_hook = MySqlHook(connection=connection, schema=self.database) # <-
self.db_hook = db_hook
return db_hook
```
Maybe you could import them internally, so you won't need them both installed to use this hook.
Reopening, feel free to contribute a PR - I'd be happy to review 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Google provider : Unnecessary imports for CloudSQL operators [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on issue #40175:
URL: https://github.com/apache/airflow/issues/40175#issuecomment-2160646099
Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.
--
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: [I] Google provider : Unnecessary imports for CloudSQL operators [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on issue #40175:
URL: https://github.com/apache/airflow/issues/40175#issuecomment-2160692679
Thanks for creating the issue, but it seems to be solved in a later version by #33783.
As a Cloud Composer user, you might consider upgrading to `composer-2.X.X-airflow-2.7.3` to handle this.
--
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