Skip to content

Set context inside templates - #33645

Merged
potiuk merged 7 commits into
apache:mainfrom
ivan-afonichkin:context-inside-templates
Aug 24, 2023
Merged

Set context inside templates#33645
potiuk merged 7 commits into
apache:mainfrom
ivan-afonichkin:context-inside-templates

Conversation

@ivan-afonichkin

@ivan-afonichkin ivan-afonichkin commented Aug 23, 2023

Copy link
Copy Markdown
Contributor

Setting the context while rendering templates.

Closes: #33217.

@ivan-afonichkin
ivan-afonichkin force-pushed the context-inside-templates branch from 6f52428 to 67c0ae0 Compare August 23, 2023 07:27
uranusjr
uranusjr reviewed Aug 24, 2023
View reviewed changes
Comment thread tests/models/test_taskinstance.py Outdated
uranusjr
uranusjr reviewed Aug 24, 2023
View reviewed changes
Comment thread tests/models/test_taskinstance.py
Comment on lines +2917 to +2918
ti._run_raw_task()
assert ti.state == State.SUCCESS

@uranusjr uranusjr Aug 24, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need to actually run the task, or can we only test the rendering part? Less moving parts if possible.

@ivan-afonichkin ivan-afonichkin Aug 24, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's implemented as setting a context for rendering templates inside _execute_task_with_callbacks. I'm not aware perhaps, but is there a way to run _execute_task_with_callbacks without actually running a task? Or do you think there is a better place to set up this context?

@ivan-afonichkin @uranusjr
Update tests/models/test_taskinstance.py
f012d65 
Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
@ivan-afonichkin

ivan-afonichkin commented Aug 24, 2023

Copy link
Copy Markdown
Contributor Author

@uranusjr thanks a lot for the review. Should someone else review it or it could be merged?

@potiuk
potiuk merged commit 9fa782f into apache:main Aug 24, 2023
@potiuk potiuk added this to the Airflow 2.7.1 milestone Aug 24, 2023
@potiuk

potiuk commented Aug 24, 2023

Copy link
Copy Markdown
Member

@uranusjr thanks a lot for the review. Should someone else review it or it could be merged?

Did. Merged. Marked for 2.7.1

@uranusjr

uranusjr commented Aug 24, 2023

Copy link
Copy Markdown
Member

I was actually wondering if the context should be set inside render_templates instead, but I guess that can be done in a later PR.

@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Aug 28, 2023
ephraimbuddy pushed a commit that referenced this pull request Aug 28, 2023
@ivan-afonichkin Ivan Afonichkin
@uranusjr @ephraimbuddy
Set context inside templates (#33645)
b0f9afb 
* Set context inside templates

---------

Co-authored-by: Ivan Afonichkin <ivanafonichkin@Ivans-MacBook-Pro.local>
Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
(cherry picked from commit 9fa782f)
@ephraimbuddy ephraimbuddy mentioned this pull request Sep 4, 2023
Closed
97 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@potiuk potiuk potiuk approved these changes
@uranusjr uranusjr uranusjr approved these changes
@kaxil kaxil Awaiting requested review from kaxil
@XD-DENG XD-DENG Awaiting requested review from XD-DENG
@ashb ashb Awaiting requested review from ashb

Assignees

No one assigned

Labels

type:bug-fix Changelog: Bug Fixes

Projects

None yet

Milestone

Airflow 2.7.1

Development

Successfully merging this pull request may close these issues.

get_current_context not present in user_defined_macros

4 participants

@ivan-afonichkin @potiuk @uranusjr @ephraimbuddy