Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change type definition for provider_info_cache decorator #39750

Merged
merged 1 commit into from
May 26, 2024

Conversation

Taragolis
Copy link
Contributor

This part decoupled from the #39430, see for details: #39430 (comment)


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@Taragolis Taragolis added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label May 22, 2024
@Taragolis Taragolis added this to the Airflow 2.9.2 milestone May 22, 2024
@Taragolis Taragolis added type:misc/internal Changelog: Misc changes that should appear in change log and removed changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) labels May 22, 2024
@potiuk potiuk added the full tests needed We need to run full set of tests for this PR to merge label May 22, 2024
@potiuk
Copy link
Member

potiuk commented May 22, 2024

Added full test needed just in case. Indeed it should work quite nice with TYPE_CHECKING now, but let's make sure all tests pass (and the STATICA_HACK was there since I remember :). Note that it also has been used in a few other places.

@Taragolis
Copy link
Contributor Author

Taragolis commented May 22, 2024

I guess STATICA_HACK grabbed originally from the celery repo / SO because there is no TYPE_CHECKING exists at that moment (added in Python 3.5.2), so it is not surprising that it might work incorrectly in static checkers and even breaks their behaviour

@Taragolis
Copy link
Contributor Author

In the other places it will be removed by #39430

@Taragolis
Copy link
Contributor Author

Only Tests / Special tests / Pydantic removed test / All:Pydantic-Removed-Postgres:12 which is not related to this changes

____ ERROR collecting tests/providers/google/cloud/links/test_translate.py _____
ImportError while importing test module '/opt/airflow/tests/providers/google/cloud/links/test_translate.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/local/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/providers/google/cloud/links/test_translate.py:31: in <module>
    from airflow.providers.google.cloud.operators.automl import (
airflow/providers/google/cloud/operators/automl.py:40: in <module>
    from airflow.providers.google.cloud.hooks.vertex_ai.prediction_service import PredictionServiceHook
airflow/providers/google/cloud/hooks/vertex_ai/prediction_service.py:24: in <module>
    from google.cloud.aiplatform_v1 import PredictionServiceClient
E   ModuleNotFoundError: No module named 'google.cloud.aiplatform_v1'
----- generated xml file: /files/test_result-providers_google-postgres.xml -----

It is also failed in main

@Taragolis
Copy link
Contributor Author

@potiuk
Copy link
Member

potiuk commented May 22, 2024

I guess STATICA_HACK grabbed originally from the celery repo / SO because there is no TYPE_CHECKING exists at that moment (added in Python 3.5.2), so it is not surprising that it might work incorrectly in static checkers and even breaks their behaviour

Yep. I think it does date back to very old times :). Glad we are fixing it now.

@potiuk potiuk force-pushed the providers-manager-typing branch 2 times, most recently from bc47d90 to d9517dd Compare May 26, 2024 20:16
@potiuk potiuk merged commit 4dffec4 into apache:main May 26, 2024
69 checks passed
RNHTTR pushed a commit to RNHTTR/airflow that referenced this pull request Jun 1, 2024
ephraimbuddy pushed a commit that referenced this pull request Jun 4, 2024
ephraimbuddy pushed a commit that referenced this pull request Jun 5, 2024
ephraimbuddy pushed a commit that referenced this pull request Jun 5, 2024
utkarsharma2 pushed a commit that referenced this pull request Jun 5, 2024
ephraimbuddy pushed a commit that referenced this pull request Jun 5, 2024
fdemiane pushed a commit to fdemiane/airflow that referenced this pull request Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full tests needed We need to run full set of tests for this PR to merge type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants