Skip to content

Conversation

@sunank200
Copy link
Collaborator

@sunank200 sunank200 commented May 12, 2025

This PR shows warnings at the top of each of the three re-export modules:
* airflow.models.param
* airflow.models.baseoperatorlink
* airflow.secrets.cache


^ 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 airflow-core/newsfragments.

@sunank200 sunank200 requested a review from amoghrajesh May 12, 2025 11:07
@sunank200 sunank200 force-pushed the backcompact-shim-af2-libraries branch from 8cebc55 to b638de5 Compare May 12, 2025 11:07
@sunank200 sunank200 marked this pull request as ready for review May 12, 2025 11:08
Copy link
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

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

We have a utility: add_deprecated_classes which can do it for us. Let's use that instead. Refer: https://github.com/apache/airflow/pull/50435/files for example

@sunank200 sunank200 requested a review from amoghrajesh May 12, 2025 13:38
@sunank200 sunank200 force-pushed the backcompact-shim-af2-libraries branch 4 times, most recently from 7a84637 to f468de2 Compare May 12, 2025 16:00
Copy link
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

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

Last set of nits. LGTM

@sunank200 sunank200 force-pushed the backcompact-shim-af2-libraries branch from cc08622 to 68d47ba Compare May 12, 2025 17:36
Copy link
Contributor

@shahar1 shahar1 left a comment

Choose a reason for hiding this comment

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

Tests fail :(

@jscheffl jscheffl added the backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch label May 12, 2025
@jscheffl
Copy link
Contributor

I propose to backport to 3.0-line!

@jscheffl jscheffl added this to the Airflow 3.0.2 milestone May 12, 2025
@potiuk
Copy link
Member

potiuk commented May 12, 2025

This is pretty unintended usage of those utils. The way how it works is that you should add those dicts and call those methods in in __init__.py of the packages which contain deprecated modules - this way we avoid having even the module files. Recently @amoghrajesh made similar assumption and he added docstring to the classes explaining it better.

https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/utils/deprecation_tools.py#L83

See for example the usage https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/decorators/__init__.py - that leaves behind only folder with __init__,py and no module files at all.

@potiuk
Copy link
Member

potiuk commented May 12, 2025

#50510 with better description of how it should be used.

@sunank200 sunank200 force-pushed the backcompact-shim-af2-libraries branch from 3fc51d1 to 36a1636 Compare May 12, 2025 20:14
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

That's it ! way less visible, and even autocomplete won't work in IDEs :)

@sunank200
Copy link
Collaborator Author

That's it ! way less visible, and even autocomplete won't work in IDEs :)

Thanks @potiuk

I did not realise the way of usage of add_deprecated_classes before.

@sunank200 sunank200 force-pushed the backcompact-shim-af2-libraries branch from 36a1636 to 5977e93 Compare May 12, 2025 20:25
@potiuk
Copy link
Member

potiuk commented May 12, 2025

That's it ! way less visible, and even autocomplete won't work in IDEs :)

Thanks @potiuk

I did not realise the way of usage of add_deprecated_classes before.

Yeah it was a bit "use by following other usages" rather than "by docs" -> I hope the update to docstring I added will make it clearer what is the intended usage :)

@sunank200 sunank200 force-pushed the backcompact-shim-af2-libraries branch 2 times, most recently from 609381a to ace83ec Compare May 13, 2025 06:58
@amoghrajesh
Copy link
Contributor

This is pretty unintended usage of those utils. The way how it works is that you should add those dicts and call those methods in in __init__.py of the packages which contain deprecated modules - this way we avoid having even the module files. Recently @amoghrajesh made similar assumption and he added docstring to the classes explaining it better.

https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/utils/deprecation_tools.py#L83

See for example the usage https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/decorators/__init__.py - that leaves behind only folder with __init__,py and no module files at all.

Ah! My bad for the review. I encountered the same issue but didn't look carefully enough.

Copy link
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

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

Alright, looks great now!

@sunank200 sunank200 force-pushed the backcompact-shim-af2-libraries branch 6 times, most recently from 549ae31 to 6f9f86f Compare May 14, 2025 06:20
@sunank200 sunank200 force-pushed the backcompact-shim-af2-libraries branch from 6f9f86f to 7cdc8ea Compare May 14, 2025 08:13
@amoghrajesh amoghrajesh merged commit d6e8418 into apache:main May 14, 2025
51 checks passed
@amoghrajesh amoghrajesh deleted the backcompact-shim-af2-libraries branch May 14, 2025 08:51
@github-actions
Copy link

Backport failed to create: v3-0-test. View the failure log Run details

Status Branch Result
v3-0-test Commit Link

You can attempt to backport this manually by running:

cherry_picker d6e8418 v3-0-test

This should apply the commit to the v3-0-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.

After you have resolved the conflicts, you can continue the backport process by running:

cherry_picker --continue

amoghrajesh pushed a commit to amoghrajesh/airflow that referenced this pull request May 14, 2025
@amoghrajesh
Copy link
Contributor

Manual cherry pick: #50588

@amoghrajesh amoghrajesh removed the backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch label May 14, 2025
@amoghrajesh
Copy link
Contributor

This can be 3.1, not an imp bug fix.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants