Skip to content

Validate AppflowBaseOperator source after template rendering - #70482

Closed
mitre88 wants to merge 2 commits into
apache:mainfrom
mitre88:fix-appflow-template-source
Closed

Validate AppflowBaseOperator source after template rendering#70482
mitre88 wants to merge 2 commits into
apache:mainfrom
mitre88:fix-appflow-template-source

Conversation

@mitre88

@mitre88 mitre88 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

source is one of AppflowBaseOperator's template fields, but it was validated against SUPPORTED_SOURCES inside __init__, which runs before Jinja rendering — so a templated source was rejected at Dag parse time with the raw expression in the error message. This moves the check to the top of execute(), where the rendered value is available, and removes the class from the exemption list.

related: #70296


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

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

source is a template field, but it was validated in __init__, which runs
before Jinja rendering — so any templated value was rejected at Dag parse
time. Validate in execute() instead, where the rendered value is available.
@boring-cyborg

boring-cyborg Bot commented Jul 27, 2026

Copy link
Copy Markdown

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example Dag that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@uranusjr

Copy link
Copy Markdown
Member

The test should also validate a templated value actually passes the test. The current test also passed without the change to implementation.

@mitre88

mitre88 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review! Added test_templated_source_passes_validation_after_rendering: the operator is constructed with source="{{ var.value.appflow_source }}" (which raised ValueError at parse time before this change) and executes successfully once the field holds the rendered value. Verified the new test fails against the previous implementation and passes with this one.

One note on scope: the AppflowRun{Full,Before,After,Daily}Operator subclasses still have their own source/filter_date checks in __init__ (they are not in the exemption list since they inherit template_fields rather than declaring them), so a templated source still fails at parse time through those. Happy to move those in a follow-up PR if that is welcome.


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

@mitre88

mitre88 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Closing — #70440 merged a more complete version of this: it moves the base-class check to execute time and also converts the AppflowRun* subclass checks to overridable _validate_source/_validate_filter_date hooks (the follow-up gap noted above). Nothing left for this PR to add.


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

@mitre88 mitre88 closed this Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants