ashb commented on code in PR #50642:
URL: https://github.com/apache/airflow/pull/50642#discussion_r2095329980
##########
airflow-core/docs/administration-and-deployment/plugins.rst:
##########
@@ -171,7 +171,7 @@ definitions in Airflow.
from airflow.providers.amazon.aws.transfers.gcs_to_s3 import
GCSToS3Operator
- # Will show up under airflow.macros.test_plugin.plugin_macro
+ # Will show up under airflow.sdk.macros.test_plugin.plugin_macro
# and in templates through {{ macros.test_plugin.plugin_macro }}
Review Comment:
```suggestion
# Will show up in templates through {{ macros.test_plugin.plugin_macro }}
```
##########
task-sdk/src/airflow/sdk/__init__.py:
##########
@@ -112,6 +114,7 @@
"task": ".definitions.decorators",
"task_group": ".definitions.decorators",
"teardown": ".definitions.decorators",
+ "macros": ".definitions.macros",
Review Comment:
I don't think we need this lazy import -- users should not be importing
macro modules directly anyway, they should only use it via `{{ macros.foo }}`
in templates, so we don't need the convince import here.
##########
airflow-core/src/airflow/plugins_manager.py:
##########
@@ -525,7 +525,7 @@ def integrate_macros_plugins() -> None:
if plugin.name is None:
raise AirflowPluginException("Invalid plugin name")
- macros_module = make_module(f"airflow.macros.{plugin.name}",
plugin.macros)
+ macros_module = make_module(f"airflow.sdk.macros.{plugin.name}",
plugin.macros)
Review Comment:
```suggestion
macros_module =
make_module(f"airflow.sdk.definitions.macros.{plugin.name}", plugin.macros)
```
Though also: macros aren't definition time, so I wonder if this should
actually be moved to:
```python
macros_module =
make_module(f"airflow.sdk.execution_time.macros.{plugin.name}", plugin.macros)
```
(and move the current `definitions.macros` to `execution_time.macros`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
