You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "jscheffl (via GitHub)" <gi...@apache.org> on 2025/12/27 09:57:10 UTC
[PR] Remove global from plaungs_manager [airflow]
jscheffl opened a new pull request, #59851:
URL: https://github.com/apache/airflow/pull/59851
Another small (in this case rather medium complex) increment to remove global statements for PR https://github.com/apache/airflow/pull/58116
This removes (most of the) global statements from plugins_manager.py where the a lot of state was hold in variables used with `global`, smelled a bit like a public interface and needed slight refactoring in other parts of the core to use proper access methods. Using a private container class as well as functools.cache() to prevent usage of global variables.
`global` is evil.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP] Remove global from plugins_manager [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #59851:
URL: https://github.com/apache/airflow/pull/59851#issuecomment-3693881973
> Why the .js changes?
Somehow the Prek plugin for JS compiler kicked-in after the small PY changes in FAB... not sure why. Seems some signature change...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove global from plugins_manager [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #59851:
URL: https://github.com/apache/airflow/pull/59851
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove global from plugins_manager [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59851:
URL: https://github.com/apache/airflow/pull/59851#discussion_r2649066585
##########
airflow-core/src/airflow/plugins_manager.py:
##########
@@ -55,30 +55,48 @@
log = logging.getLogger(__name__)
-import_errors: dict[str, str] = {}
-
-plugins: list[AirflowPlugin] | None = None
-loaded_plugins: set[str] = set()
-
-# Plugin components to integrate as modules
-macros_modules: list[Any] | None = None
-
-# Plugin components to integrate directly
-admin_views: list[Any] | None = None
-flask_blueprints: list[Any] | None = None
-fastapi_apps: list[Any] | None = None
-fastapi_root_middlewares: list[Any] | None = None
-external_views: list[Any] | None = None
-react_apps: list[Any] | None = None
-menu_links: list[Any] | None = None
-flask_appbuilder_views: list[Any] | None = None
-flask_appbuilder_menu_links: list[Any] | None = None
-global_operator_extra_links: list[Any] | None = None
-operator_extra_links: list[Any] | None = None
-registered_operator_link_classes: dict[str, type] | None = None
-timetable_classes: dict[str, type[Timetable]] | None = None
-hook_lineage_reader_classes: list[type[HookLineageReader]] | None = None
-priority_weight_strategy_classes: dict[str, type[PriorityWeightStrategy]] | None = None
+
+class _PluginsManagerState:
Review Comment:
Nice!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove global from plugins_manager [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #59851:
URL: https://github.com/apache/airflow/pull/59851#issuecomment-3693880962
Why the .js changes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove global from plugins_manager [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #59851:
URL: https://github.com/apache/airflow/pull/59851#discussion_r2649067499
##########
airflow-core/src/airflow/plugins_manager.py:
##########
@@ -55,30 +55,48 @@
log = logging.getLogger(__name__)
-import_errors: dict[str, str] = {}
-
-plugins: list[AirflowPlugin] | None = None
-loaded_plugins: set[str] = set()
-
-# Plugin components to integrate as modules
-macros_modules: list[Any] | None = None
-
-# Plugin components to integrate directly
-admin_views: list[Any] | None = None
-flask_blueprints: list[Any] | None = None
-fastapi_apps: list[Any] | None = None
-fastapi_root_middlewares: list[Any] | None = None
-external_views: list[Any] | None = None
-react_apps: list[Any] | None = None
-menu_links: list[Any] | None = None
-flask_appbuilder_views: list[Any] | None = None
-flask_appbuilder_menu_links: list[Any] | None = None
-global_operator_extra_links: list[Any] | None = None
-operator_extra_links: list[Any] | None = None
-registered_operator_link_classes: dict[str, type] | None = None
-timetable_classes: dict[str, type[Timetable]] | None = None
-hook_lineage_reader_classes: list[type[HookLineageReader]] | None = None
-priority_weight_strategy_classes: dict[str, type[PriorityWeightStrategy]] | None = None
+
+class _PluginsManagerState:
Review Comment:
Note: Will rework and remove code further. hold on for extensice review :-D - is WIP
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP] Remove global from plugins_manager [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #59851:
URL: https://github.com/apache/airflow/pull/59851#issuecomment-3693889709
> > Why the .js changes?
>
> Somehow the Prek plugin for JS compiler kicked-in after the small PY changes in FAB... not sure why. Seems some signature change...
This one changed:
```
providers/fab/src/airflow/providers/fab/www/extensions/__init__.py
```
And it matches "www" of the provider:
```
"www": PROVIDERS_ROOT / "fab" / "src" / "airflow" / "providers" / "fab" / "www",
```
We should probably make it a bit more selective when we switch to `hatch` for both `edge3` and `fab` - but it's **okish** as it is now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove global from plugins_manager [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #59851:
URL: https://github.com/apache/airflow/pull/59851#issuecomment-3694911820
> We should probably make it a bit more selective when we switch to `hatch` for both `edge3` and `fab` - but it's **okish** as it is now.
Changes the config in this PR now as the JS changes generated false alarms and this bothered me. Ready for review now!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org