-
Notifications
You must be signed in to change notification settings - Fork 16.3k
[AIRFLOW-4451] Allow templated named tuples #5673
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
[AIRFLOW-4451] Allow templated named tuples #5673
Conversation
9a1ffda to
11c67a7
Compare
|
The kube git test is failing for what looks like a timeout, not sure how to restart it. |
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.
Would it be too much to ask you to separate out the test for template and add jinja template to the value of the named tuple in those tests (maybe alongside adding dict, list etc.).
|
@potiuk I can definitely move those test cases into a separate test, as well as ensure that the named tuple values are actually rendered too. I'm happy to add those cases to any base template rendering tests that exist instead, do you know where those might be located? |
|
airflow/tests/models/test_dag.py Line 391 in b5fb370
Ha, answered my own question! I'll add a test case here for named tuples. |
ashb
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.
Your PR is failing Flake8 and mypy tests - please ensure these at least are passing locally before pushing your next commit/fixup.
5152301 to
35c2a62
Compare
ashb
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.
[AIRFLOW-4451] Fix separate checks for rendering lists & tuples Co-Authored-By: Bas Harenslak <[email protected]>
72c91bc to
4dca2df
Compare
|
LGTM, we can merge once CI is successful. |
|
4995a67 to
76cfeeb
Compare
|
Tests passing! 🎉 |
|
Thanks all for the help! Glad we'll be able to get this in 1.10.4 😁 |
(cherry picked from commit 6e425f4)
(cherry picked from commit 6e425f4)
Make sure you have checked all steps below.
Jira
Description
PR #4691 introduced an issue where named tuples were inadvertently converted to a list of rendered entities. This then made subsequent attribute access on what was assumed to be a named tuple fail, since the object was actually a list.
Example:
This PR adds an additional clause in the base render function to catch named tuples, and re-instantiate them as the proper class after variable rendering.
Tests
This adds another clause to the python operator tests to ensure that rendered
op_argsthat are named tuples are still preserved.Commits
Documentation
Code Quality
flake8