Skip to content

Copy Self to observability shared package#59140

Merged
potiuk merged 1 commit into
apache:mainfrom
potiuk:remove-airflow-import-from-observability
Dec 6, 2025
Merged

Copy Self to observability shared package#59140
potiuk merged 1 commit into
apache:mainfrom
potiuk:remove-airflow-import-from-observability

Conversation

@potiuk
Copy link
Copy Markdown
Member

@potiuk potiuk commented Dec 6, 2025

Shared packages should not refer to airflow or airflow.sdk. This is the last "airflow" import remaining in the observability package.

Fixed during implementation of #58825


^ 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.

Shared packages should not refer to airflow or airflow.sdk. This
is the last "airflow" import remaining in the observability package.

Fixed during implementation of apache#58825
@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Dec 6, 2025

cc: @xBis7

Copy link
Copy Markdown
Contributor

@xBis7 xBis7 left a comment

Choose a reason for hiding this comment

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

Nice and simple fix. It makes sense to duplicate the code since I can see that only observability uses it from shared. LGTM!

@amoghrajesh
Copy link
Copy Markdown
Contributor

Since its only used for type checking, cant we just add the "typing-extensions" path?

@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented Dec 6, 2025

Since its only used for type checking, cant we just add the "typing-extensions" path?

I think not - I prefer not to depend on "typing-extensions" version of "Self" when we are already on Python 3.11+ in this case. Also having it this way makes it easy to have "rolling deprecation removal". When we move to Python 3.11+ next October - we will know we have to remove this completely. If we import from typing_extensions - there is no obvious signal that we should remove this one.

@potiuk potiuk changed the title Move Self to observability shared package Copy Self to observability shared package Dec 6, 2025
@potiuk potiuk merged commit 0ee0daf into apache:main Dec 6, 2025
56 checks passed
@potiuk potiuk deleted the remove-airflow-import-from-observability branch December 6, 2025 22:05
@amoghrajesh
Copy link
Copy Markdown
Contributor

Since its only used for type checking, cant we just add the "typing-extensions" path?

I think not - I prefer not to depend on "typing-extensions" version of "Self" when we are already on Python 3.11+ in this case. Also having it this way makes it easy to have "rolling deprecation removal". When we move to Python 3.11+ next October - we will know we have to remove this completely. If we import from typing_extensions - there is no obvious signal that we should remove this one.

Sounds ok, thanks.

amoghrajesh pushed a commit to astronomer/airflow that referenced this pull request Dec 8, 2025
Shared packages should not refer to airflow or airflow.sdk. This
is the last "airflow" import remaining in the observability package.

Fixed during implementation of apache#58825
jhgoebbert pushed a commit to jhgoebbert/airflow_Owen-CH-Leung that referenced this pull request Feb 8, 2026
Shared packages should not refer to airflow or airflow.sdk. This
is the last "airflow" import remaining in the observability package.

Fixed during implementation of apache#58825
Subham-KRLX pushed a commit to Subham-KRLX/airflow that referenced this pull request Mar 4, 2026
Shared packages should not refer to airflow or airflow.sdk. This
is the last "airflow" import remaining in the observability package.

Fixed during implementation of apache#58825
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.

4 participants