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

plugins: load plugins from providers #32692

Merged
merged 1 commit into from
Jul 25, 2023
Merged

Conversation

mobuchowski
Copy link
Contributor

In some cases - like when provider is included by breeze prod-image build --install-providers-from-sources plugins are not loaded, as they are not separate package with entrypoint then. This PR adds loading plugins from provider.yml. I think this is somehow similar to what @potiuk is doing with stuff like config or executors.

In some cases this would make entrypoint plugins double-load - so we check if the plugin is already loaded. We could avoid that by not generating entrypoints for plugins, but this would make them not load on earlier versions of Airflow.

@potiuk
Copy link
Member

potiuk commented Jul 19, 2023

Oh yeah. Good point. I briefly looked at it - looks like the right direction - I think there is a bit of a problem of where the plugins are loaded from during tests as some tests expect to load plugins from "example_dags/plugins" and I think this PR removes that path somehow,

It's going to be much clearer and more straightforward after #32669 will be merged, because one of the things it does is that it unifies the unit test (local venv) / and CI environment for tesst so that it is configured in one place only and dynamically loaded via pytest fixture from here:

https://github.com/apache/airflow/pull/32669/files#diff-1748727a9777b78ae8c17034fa1c5d4a4fd38de8ce986ef13e6531b347f65588R41

So it might be a wise thing to wait after #32669 is merged, rebase and try to fix it there (happy to help/brainstorm - I got quite a bit more familiar with how unit test configuration is loaded by implementing the PRs)

@potiuk
Copy link
Member

potiuk commented Jul 19, 2023

not generating entrypoints for plugins, but this would make them not load on earlier versions of Airflow

BTW. We can't do it even for the future - because when Airflow gets providers installed as packages, using entrypoints is the only mechanism how it can discover plugins, there are no provider.yaml files any more in packaged version of Airflow (and can't be because providers are isolated and independent from Airflow and there might me different versions of both - community and 3rd-party providers installed together with Airflow, so the only way Airflow can find out what plugins are availabe is by querying the entrypoints.

@mobuchowski mobuchowski force-pushed the load-plugins-from-providers branch 2 times, most recently from b78c4ce to e6bd521 Compare July 24, 2023 10:35
airflow/plugins_manager.py Outdated Show resolved Hide resolved
airflow/providers_manager.py Outdated Show resolved Hide resolved
@mobuchowski mobuchowski force-pushed the load-plugins-from-providers branch 3 times, most recently from 1332b89 to 46f0ce1 Compare July 24, 2023 19:28
Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com>
@mobuchowski mobuchowski force-pushed the load-plugins-from-providers branch from 46f0ce1 to 1613bdb Compare July 24, 2023 20:01
@mobuchowski mobuchowski added full tests needed We need to run full set of tests for this PR to merge and removed full tests needed We need to run full set of tests for this PR to merge labels Jul 24, 2023
@mobuchowski mobuchowski merged commit a23bf4c into main Jul 25, 2023
@ephraimbuddy ephraimbuddy added the type:new-feature Changelog: New Features label Aug 2, 2023
@ephraimbuddy ephraimbuddy added this to the Airflow 2.7.0 milestone Aug 2, 2023
@Taragolis Taragolis deleted the load-plugins-from-providers branch September 26, 2023 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:plugins type:new-feature Changelog: New Features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants