Skip to content

Resolve GCSFileTransformOperator destination fallbacks after rendering - #70488

Open
mitre88 wants to merge 1 commit into
apache:mainfrom
mitre88:fix-gcs-operators-template
Open

Resolve GCSFileTransformOperator destination fallbacks after rendering#70488
mitre88 wants to merge 1 commit into
apache:mainfrom
mitre88:fix-gcs-operators-template

Conversation

@mitre88

@mitre88 mitre88 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Part of the template-field validation burn-down tracked in #70296.

GCSFileTransformOperator resolves destination_bucket/destination_object fallbacks (destination_bucket or source_bucket) in __init__, where the source fields still hold un-rendered Jinja expressions. This moves the fallback resolution to execute() (and mirrors it in get_openlineage_facets_on_start, which runs before execute), so the fallback applies to the rendered values.

Scope notes from review:

  • The delimiter deprecation warning in GCSListObjectsOperator stays in __init__ — deprecation applies to any usage of the argument, not the rendered value.
  • The objects/prefix provision checks in GCSDeleteObjectsOperator stay in __init__ — they are false positives under the relaxed validator definition proposed in Allow argument-provision checks of templated fields in operator __init__ #70505. Both classes keep their exemption entries; only the GCSFileTransformOperator entry is removed.

Note: TestGCSDeleteObjectsOperator::test_get_openlineage_facets_on_start[objects] fails on a pristine main checkout as well — pre-existing and unrelated to this change.


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Fable 5)

Generated-by: Claude Code (Fable 5) following the guidelines

Comment thread providers/google/src/airflow/providers/google/cloud/operators/gcs.py Outdated

@shahar1 shahar1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please focus this PR only on GCSFileTransformOperator (see ##70505 for more details)

Comment on lines -321 to -331
if objects is None and prefix is None:
err_message = "(Task {task_id}) Either objects or prefix should be set. Both are None.".format(
**kwargs
)
raise ValueError(err_message)
if objects is not None and prefix is not None:
err_message = "(Task {task_id}) Objects or prefix should be set. Both provided.".format(**kwargs)
raise ValueError(err_message)

super().__init__(**kwargs)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please restore the changes here, as it will be flagged as false-positive (see ##70505 for more details)

raise ValueError(f"(Task {self.task_id}) Either objects or prefix should be set. Both are None.")
if self.objects is not None and self.prefix is not None:
raise ValueError(f"(Task {self.task_id}) Objects or prefix should be set. Both provided.")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please revert these changes

providers/google/src/airflow/providers/google/cloud/operators/dataproc.py::DataprocCreateClusterOperator
providers/google/src/airflow/providers/google/cloud/operators/dataproc.py::DataprocSubmitJobOperator
providers/google/src/airflow/providers/google/cloud/operators/functions.py::CloudFunctionDeployFunctionOperator
providers/google/src/airflow/providers/google/cloud/operators/gcs.py::GCSDeleteObjectsOperator

@shahar1 shahar1 Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please restore this line

@mitre88 mitre88 changed the title Apply GCS operator template-field logic after rendering Resolve GCSFileTransformOperator destination fallbacks after rendering Jul 27, 2026
@mitre88

mitre88 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Done — reverted the GCSDeleteObjectsOperator changes (code, test, and exemption entry restored to match main) per the relaxed provision-check definition in #70505. The PR now only moves the GCSFileTransformOperator destination fallbacks to execute time; title and description updated accordingly.


Drafted-by: Claude Code (Fable 5) (no human review before posting)

destination_bucket/destination_object are template fields, so resolving
their 'or source_*' fallbacks in __init__ reads the unrendered value.
Move the fallback resolution to execute() where the rendered values are
available, keep get_openlineage_facets_on_complete resolving fallbacks
locally (it can run outside execute), and remove the operator from the
validate_operators_init exemptions list.

Related: apache#70296
@mitre88
mitre88 force-pushed the fix-gcs-operators-template branch from 6963c2d to 9784313 Compare July 31, 2026 22:21
@mitre88

mitre88 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

@shahar1 Done — the PR is now scoped to GCSFileTransformOperator only and rebased on current main (which includes #70505).

To confirm the remaining change is consistent with the #70505 distinction: destination_bucket or self.source_bucket is a truthiness/value read of the unrendered template parameter, not an argument-provision check (is None/is not None), so it is not covered by the new sanctioned pattern — the exemption entry only stays valid if the fallback moves out of __init__. That's what this PR does:

  • __init__ now stores destination_bucket/destination_object verbatim (no read of the unrendered value).
  • The or source_* fallback resolves in execute(), after rendering.
  • get_openlineage_facets_on_complete (which can run outside execute) resolves the fallbacks locally.
  • The GCSFileTransformOperator exemption entry is removed; validate_operators_init.py passes on the file (exit 0).

Everything else from the earlier revision (GCSDeleteObjectsOperator, GCSListObjectsOperator, their exemption entries) was dropped.


Drafted-by: Claude Code (Fable 5) (no human review before posting)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dev-tools area:providers backport-to-v3-3-test Backport to v3-3-test provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants