Skip to content

Conversation

@tatiana
Copy link
Contributor

@tatiana tatiana commented Feb 27, 2025

Since apache-airflow-providers-common-sql==1.23.0, released on 26 February 2025, users started facing the error:

    from cosmos.operators._asynchronous.bigquery import DbtRunAirflowAsyncBigqueryOperator
cosmos/operators/_asynchronous/bigquery.py:8: in <module>
    from airflow.providers.google.cloud.operators.bigquery import BigQueryInsertJobOperator
../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/google/cloud/operators/bigquery.py:32: in <module>
    from airflow.providers.common.sql.operators.sql import (  # type: ignore[attr-defined] # for _parse_boolean
../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/common/sql/operators/sql.py:[29](https://github.com/astronomer/astronomer-cosmos/actions/runs/13544957799/job/37854234771#step:6:30): in <module>
    from airflow.providers.common.sql.hooks.sql import DbApiHook, fetch_all_handler, return_single_query_results
../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/common/sql/hooks/sql.py:37: in <module>
    from methodtools import lru_cache
E   ModuleNotFoundError: No module named 'methodtools'

When trying to import:

from airflow.providers.google.cloud.operators.bigquery import BigQueryInsertJobOperator

It seems this started happening since #41327.

This PR aims to solve this issue.

Closes: #47147

@dabla
Copy link
Contributor

dabla commented Feb 27, 2025

Indeed since 41327 I've added the lru cache for use with dialects. Isn't there a way to detect such missing dependency when running the tests of the provider?

@gopidesupavan
Copy link
Member

gopidesupavan commented Feb 27, 2025

I believe we moved adding dependency to provider info and pyproject.toml files, so IMHO It would be better to add this dependency to pyproject.toml https://github.com/apache/airflow/blob/main/providers/common/sql/pyproject.toml#L57

Then the pre-commit will add that to readme and provider info file https://github.com/apache/airflow/blob/main/providers/common/sql/src/airflow/providers/common/sql/get_provider_info.py

@potiuk
Copy link
Member

potiuk commented Feb 27, 2025

I believe we moved adding dependency to provider info and pyproject.toml files, so IMHO It would be better to add this dependency to pyproject.toml https://github.com/apache/airflow/blob/main/providers/common/sql/pyproject.toml#L57

Yep. This is the new (much more standard) way how we should add dependencies. In fact I am a little puzzled why this PR did not fail with validation of the provider.yaml, because ... it should ... We do not exapect dependencies field in provider.yaml any more. Let me see why it did not fail.

@potiuk
Copy link
Member

potiuk commented Feb 27, 2025

OK. I know ... Missing case for selective checks

@potiuk
Copy link
Member

potiuk commented Feb 27, 2025

The #47148 improves selective checks to cover that case.

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes to avoid accidental merge.

@tatiana tatiana changed the title Add methodtools as a dependency of the common-sql provider Add missing methodtools dependency of the common-sql provider Feb 27, 2025
Since `apache-airflow-providers-common-sql==1.23.0`, released on 26 February 2025, users started facing the error:

```
    from cosmos.operators._asynchronous.bigquery import DbtRunAirflowAsyncBigqueryOperator
cosmos/operators/_asynchronous/bigquery.py:8: in <module>
    from airflow.providers.google.cloud.operators.bigquery import BigQueryInsertJobOperator
../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/google/cloud/operators/bigquery.py:32: in <module>
    from airflow.providers.common.sql.operators.sql import (  # type: ignore[attr-defined] # for _parse_boolean
../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/common/sql/operators/sql.py:[29](https://github.com/astronomer/astronomer-cosmos/actions/runs/13544957799/job/37854234771#step:6:30): in <module>
    from airflow.providers.common.sql.hooks.sql import DbApiHook, fetch_all_handler, return_single_query_results
../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/common/sql/hooks/sql.py:37: in <module>
    from methodtools import lru_cache
E   ModuleNotFoundError: No module named 'methodtools'
```

It seems this started happening since apache#41327.

This PR aims to solve this issue.

Closes: apache#47147
@tatiana tatiana force-pushed the fix-common-sql-methodtools branch from 427189e to e7a6619 Compare February 27, 2025 18:04
@tatiana
Copy link
Contributor Author

tatiana commented Feb 27, 2025

Thank you very much, @dabla, @gopidesupavan, and @potiuk, for the rapid reviews and feedback!

I moved the dependency from provider.yaml to pyproject.toml in e7a6619.

I noticed the following files were modified after running the pre-commit checks:

generated/provider_dependencies.json
providers/common/sql/README.rst
providers/common/sql/src/airflow/providers/common/sql/get_provider_info.py

For now, I have added them to the PR. Please let me know if that's correct - I can remove them if necessary.

@potiuk
Copy link
Member

potiuk commented Feb 27, 2025

For now, I have added them to the PR. Please let me know if that's correct - I can remove them if necessary.

Yep. This is exactly what pre-commits are doing - take your change and apply whatever resulting changes elsewhere are needed so that you do not have to even think about it.

@dabla
Copy link
Contributor

dabla commented Feb 27, 2025

Awesome to see how quickly this issue has been resolved 👍

@potiuk
Copy link
Member

potiuk commented Feb 27, 2025

The static-checks error is likely from main

@potiuk
Copy link
Member

potiuk commented Feb 27, 2025

The static check failure handled in #47172

@potiuk potiuk merged commit c0719fe into apache:main Feb 27, 2025
145 of 146 checks passed
Sharashchandra pushed a commit to Sharashchandra/airflow that referenced this pull request Feb 28, 2025
…ache#47148)

* Add methodtools as a dependency of the common-sql provider

Since `apache-airflow-providers-common-sql==1.23.0`, released on 26 February 2025, users started facing the error:

```
    from cosmos.operators._asynchronous.bigquery import DbtRunAirflowAsyncBigqueryOperator
cosmos/operators/_asynchronous/bigquery.py:8: in <module>
    from airflow.providers.google.cloud.operators.bigquery import BigQueryInsertJobOperator
../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/google/cloud/operators/bigquery.py:32: in <module>
    from airflow.providers.common.sql.operators.sql import (  # type: ignore[attr-defined] # for _parse_boolean
../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/common/sql/operators/sql.py:[29](https://github.com/astronomer/astronomer-cosmos/actions/runs/13544957799/job/37854234771#step:6:30): in <module>
    from airflow.providers.common.sql.hooks.sql import DbApiHook, fetch_all_handler, return_single_query_results
../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/common/sql/hooks/sql.py:37: in <module>
    from methodtools import lru_cache
E   ModuleNotFoundError: No module named 'methodtools'
```

It seems this started happening since apache#41327.

This PR aims to solve this issue.

Closes: apache#47147

* Fix linting

* Revert change to provider.yaml and applyc hange to pyproject.toml
nailo2c pushed a commit to nailo2c/airflow that referenced this pull request Apr 4, 2025
…ache#47148)

* Add methodtools as a dependency of the common-sql provider

Since `apache-airflow-providers-common-sql==1.23.0`, released on 26 February 2025, users started facing the error:

```
    from cosmos.operators._asynchronous.bigquery import DbtRunAirflowAsyncBigqueryOperator
cosmos/operators/_asynchronous/bigquery.py:8: in <module>
    from airflow.providers.google.cloud.operators.bigquery import BigQueryInsertJobOperator
../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/google/cloud/operators/bigquery.py:32: in <module>
    from airflow.providers.common.sql.operators.sql import (  # type: ignore[attr-defined] # for _parse_boolean
../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/common/sql/operators/sql.py:[29](https://github.com/astronomer/astronomer-cosmos/actions/runs/13544957799/job/37854234771#step:6:30): in <module>
    from airflow.providers.common.sql.hooks.sql import DbApiHook, fetch_all_handler, return_single_query_results
../../../.local/share/hatch/env/virtual/astronomer-cosmos/Za_bFbg4/tests.py3.12-2.9/lib/python3.12/site-packages/airflow/providers/common/sql/hooks/sql.py:37: in <module>
    from methodtools import lru_cache
E   ModuleNotFoundError: No module named 'methodtools'
```

It seems this started happening since apache#41327.

This PR aims to solve this issue.

Closes: apache#47147

* Fix linting

* Revert change to provider.yaml and applyc hange to pyproject.toml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Common SQL provider depends on the methodtools package

4 participants