Skip to content
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

Deprecate conf from Task Context #44968

Merged
merged 2 commits into from
Dec 17, 2024
Merged

Conversation

kaxil
Copy link
Member

@kaxil kaxil commented Dec 16, 2024

This was initially added in response to #168. However, we now have ti.log_url that is used for that; example usages:

"Exception:<br>{{exception_html}}<br>"

<td><a href="{{ ti.log_url }}" style="text-decoration:underline;">{{ ti.log_url }}</a></td>

Log: <a href="{{ti.log_url}}">Link</a><br>

So, to simplify what we need to pass from API server to the Task SDK in preparation for Airflow 3, I want to simplify and remove things that aren't needed. In this case, this is good so we don't pass/expore secrets unnecesarily via conf. This is removed in Airflow 3 and deprecated in 2.10.x/2.11

Mailing list Thread: https://lists.apache.org/thread/2n0l8y2oyq4442p0lsnmbbcl6rmbj3k7

PR for Airflow 3: #44820


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

This was initially added in response to apache#168. However, we now have `ti.log_url` that is used for that; example usages:

https://github.com/apache/airflow/blob/dcd41f60f1c9b5583b49bfb49b6d85c640a2892c/airflow/models/taskinstance.py#L1362

https://github.com/apache/airflow/blob/dcd41f60f1c9b5583b49bfb49b6d85c640a2892c/providers/src/airflow/providers/smtp/notifications/templates/email.html#L28

https://github.com/apache/airflow/blob/dcd41f60f1c9b5583b49bfb49b6d85c640a2892c/docs/apache-airflow/howto/email-config.rst?plain=1#L76

So, to simplify what we need to pass from API server to the Task SDK in preparation for Airflow 3, I want to simplify and remove things that aren't needed. In this case, this is good so we don't pass/expore secrets unnecesarily via `conf`. This is removed in Airflow 3 and deprecated in 2.10.x/2.11

Mailing list Thread: https://lists.apache.org/thread/2n0l8y2oyq4442p0lsnmbbcl6rmbj3k7
@kaxil kaxil added this to the Airflow 2.10.5 milestone Dec 16, 2024
@kaxil kaxil requested review from uranusjr and jscheffl December 16, 2024 19:43
@kaxil
Copy link
Member Author

kaxil commented Dec 16, 2024

@jscheffl / @uranusjr -- Does this good look for 2.10.5/2.11?

Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

Look good.

Can you please also add a deprecation note to https://github.com/apache/airflow/blob/v2-10-test/docs/apache-airflow/templates-ref.rst#deprecated-variables ?

Newsfragment would also be good!

@kaxil
Copy link
Member Author

kaxil commented Dec 16, 2024

Look good.

Can you please also add a deprecation note to https://github.com/apache/airflow/blob/v2-10-test/docs/apache-airflow/templates-ref.rst#deprecated-variables ?

Newsfragment would also be good!

Done in a28a118

@kaxil kaxil requested a review from potiuk as a code owner December 16, 2024 20:16
@uranusjr
Copy link
Member

Should this target 2.11.0 instead? It may be weird for users if a deprecation warning starts showing up when they do a patch version upgrade.

@jscheffl
Copy link
Contributor

Should this target 2.11.0 instead? It may be weird for users if a deprecation warning starts showing up when they do a patch version upgrade.

We had this for other places as well - if we know about a deprecation we should inform users as soon as we know. I'd also propose to not hold-back the information about upcoming deprecation.

@kaxil kaxil merged commit 02a1be8 into apache:v2-10-test Dec 17, 2024
48 checks passed
@kaxil kaxil deleted the deprecate-conf branch December 17, 2024 10:59
@kaxil kaxil restored the deprecate-conf branch December 17, 2024 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants