Skip to content

Conversation

@kaxil
Copy link
Member

@kaxil kaxil commented Oct 23, 2024

We had removed registering hooks via plugin in #12108 for Airflow 2.0. We kept it for registering Connection form.

As I understand it, we don't use that code but ProvidersManager (airflow/providers_manager.py) to register Connections from providers.

Our docs already mark this as removed in 2.0:

image

.. versionchanged:: 2.0
Importing operators, sensors, hooks added in plugins via
``airflow.{operators,sensors,hooks}.<plugin_name>`` is no longer supported, and these extensions should
just be imported as regular python modules. For more information, see: :doc:`../administration-and-deployment/modules_management` and
:doc:`/howto/custom-operator`


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

@kaxil
Copy link
Member Author

kaxil commented Oct 23, 2024

@ashb since you were the author of #12108 , could you review this PR and see if it makes sense

@kaxil kaxil force-pushed the remove-hooks-plugin branch from 1e7cc79 to a6b28d4 Compare October 23, 2024 02:05
@potiuk
Copy link
Member

potiuk commented Oct 23, 2024

Should be fine. The plugins from Airlfow 2 will stop working anyway and people will have to modify them. This is pretty much a "given" incompatibility and we need to bite the bullet. But the test needs to be fixed and I think we should add a newsfragment (just one liner now and we can expand it later).

@kaxil kaxil force-pushed the remove-hooks-plugin branch from a6b28d4 to 12e96bd Compare October 23, 2024 11:05
@kaxil
Copy link
Member Author

kaxil commented Oct 23, 2024

Should be fine. The plugins from Airlfow 2 will stop working anyway and people will have to modify them. This is pretty much a "given" incompatibility and we need to bite the bullet. But the test needs to be fixed and I think we should add a newsfragment (just one liner now and we can expand it later).

That was already part of the docs though, check PR description -- should we still add a newsfragment?

Our docs already mark this as removed in 2.0:

image

.. versionchanged:: 2.0
Importing operators, sensors, hooks added in plugins via
``airflow.{operators,sensors,hooks}.<plugin_name>`` is no longer supported, and these extensions should
just be imported as regular python modules. For more information, see: :doc:`../administration-and-deployment/modules_management` and
:doc:`/howto/custom-operator`

Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

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

Yeah looks good to me.

I think an explicit news fragment is a good idea, even though the docs say it was deprecated this should be called out as finally breaking. (I'm thinking to the issue FAB/WTForms had recently, so lets be explicit when we can)

@kaxil kaxil force-pushed the remove-hooks-plugin branch from b7fc435 to 3f92bf7 Compare October 23, 2024 13:01
@kaxil
Copy link
Member Author

kaxil commented Oct 23, 2024

Added in 3f92bf7

@kaxil kaxil force-pushed the remove-hooks-plugin branch 2 times, most recently from 23740a9 to 8f6360c Compare October 23, 2024 15:16
We had removed registering hooks via plugin in apache#12108 for Airflow 2.0. We kept it for registering Connection form.

As I understand it, we don't use that code but `ProvidersManager` (`airflow/providers_manager.py`) to register Connections from providers.
@kaxil kaxil force-pushed the remove-hooks-plugin branch from 8f6360c to e46b311 Compare October 23, 2024 15:43
@kaxil kaxil merged commit 2bf3af0 into apache:main Oct 23, 2024
@kaxil kaxil deleted the remove-hooks-plugin branch October 23, 2024 16:24
harjeevanmaan pushed a commit to harjeevanmaan/airflow that referenced this pull request Oct 23, 2024
We had removed registering hooks via plugin in apache#12108 for Airflow 2.0. We kept it for registering Connection form.

As I understand it, we don't use that code but `ProvidersManager` (`airflow/providers_manager.py`) to register Connections from providers.
PaulKobow7536 pushed a commit to PaulKobow7536/airflow that referenced this pull request Oct 24, 2024
We had removed registering hooks via plugin in apache#12108 for Airflow 2.0. We kept it for registering Connection form.

As I understand it, we don't use that code but `ProvidersManager` (`airflow/providers_manager.py`) to register Connections from providers.
ellisms pushed a commit to ellisms/airflow that referenced this pull request Nov 13, 2024
We had removed registering hooks via plugin in apache#12108 for Airflow 2.0. We kept it for registering Connection form.

As I understand it, we don't use that code but `ProvidersManager` (`airflow/providers_manager.py`) to register Connections from providers.
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.

4 participants