-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Simplify version-specific imports in the Google provider #56793
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
Conversation
jason810496
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it means that all providers should leverage new common.compact.lazy_compact instead of their provider specific version_compat module?
If so, will we create meta issue for migration and invite the community to help?
gopidesupavan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
potiuk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment about bumping common.compat to make @eladkal live easier. We can thing of other ways but currently we should avoid bumping version by other people than release manager.
Once we get this one in as an example I guess :) ? |
…y_compat Simplify version-specific imports in the Google provider by using lazy_compat for components that are available since Airflow 3.0. This reduces code duplication and leverages the centralized compatibility layer. Changes: - version_compat.py: Use lazy_compat for BaseOperatorLink, BaseSensorOperator, PokeReturnValue, timeout - cloud/links/base.py: Use lazy_compat for XCom import - marketing_platform/links/analytics_admin.py: Use lazy_compat for BaseOperatorLink and XCom Note: BaseHook and BaseOperator imports in version_compat.py remain unchanged as they require AIRFLOW_V_3_1_PLUS conditional logic (not 3.0) due to SDK compatibility. Benefits: - Removed try/except import logic for timeout - Removed if/else blocks for SDK components - Consistent compatibility imports across providers Remove lazy_compat re-exports from Google version_compat.py Remove BaseOperatorLink, BaseSensorOperator, PokeReturnValue, and timeout from version_compat.py. Files that need these imports now import them directly from airflow.providers.common.compat.lazy_compat. This simplifies version_compat.py to only handle version constants and the BaseHook/BaseOperator imports that require special 3.1+ logic. Changes: - version_compat.py: Removed lazy_compat re-exports - bigquery.py sensor: Import BaseSensorOperator from lazy_compat - dataflow.py sensor: Import BaseSensorOperator, PokeReturnValue from lazy_compat - dataproc.py links: Import BaseOperatorLink from lazy_compat - dataflow.py hook: Import timeout from lazy_compat Benefits: - Clearer separation of concerns - Direct imports show the true source of compatibility logic - version_compat.py focuses only on version-specific logic
b8c1033 to
dec80cf
Compare
Not yet :) But eventually after this PR is merged and probably one or two critical ones. |
Yeah.... Do it once, do it twice, figure out if repeatable and document -> then fire it in massive parallel way. Parallelising too early might result in parallleling wrong solution. |
Simplify version-specific imports in the Google provider by using lazy_compat from common.compat provider added in apache#56790 for components that are available since Airflow 3.0.
Consolidate the Standard provider's compatibility logic by replacing imports from version_compat.py with direct imports from lazy_compat, providing a single source of truth for all Airflow 2.x ↔ 3.x compatibility. Follow-up of apache#56790 & apache#56793
Consolidate the Standard provider's compatibility logic by replacing imports from version_compat.py with direct imports from lazy_compat, providing a single source of truth for all Airflow 2.x ↔ 3.x compatibility. Follow-up of apache#56790 & apache#56793
Simplify version-specific imports in the Google provider by using lazy_compat from common.compat provider added in apache#56790 for components that are available since Airflow 3.0.
Consolidate the Standard provider's compatibility logic by replacing imports from version_compat.py with direct imports from lazy_compat, providing a single source of truth for all Airflow 2.x ↔ 3.x compatibility. Follow-up of apache#56790 & apache#56793
Consolidate the Standard provider's compatibility logic by replacing imports from version_compat.py with direct imports from lazy_compat, providing a single source of truth for all Airflow 2.x ↔ 3.x compatibility. Follow-up of apache#56790 & apache#56793
Consolidate the Standard provider's compatibility logic by replacing imports from version_compat.py with direct imports from lazy_compat, providing a single source of truth for all Airflow 2.x ↔ 3.x compatibility. Follow-up of apache#56790 & apache#56793
) Consolidate the Standard provider's compatibility logic by replacing imports from version_compat.py with direct imports from lazy_compat, providing a single source of truth for all Airflow 2.x ↔ 3.x compatibility. Follow-up of apache#56790 & apache#56793
| from airflow.sdk.execution_time.xcom import XCom | ||
| else: | ||
| from airflow.models.xcom import XCom # type: ignore[no-redef] | ||
| from airflow.providers.common.compat.lazy_compat import BaseOperatorLink, BaseSensorOperator, XCom |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we not import BaseOperator here too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, BaseOperator is special cased for 3.1 due to xcom_push bug -- there should be a comment about it in version_compat file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember now :)
Simplify version-specific imports in the Google provider by using lazy_compat from common.compat provider added in apache#56790 for components that are available since Airflow 3.0.
Simplify version-specific imports in the Google provider by using lazy_compat from common.compat provider added in #56790
for components that are available since Airflow 3.0.