You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "amoghrajesh (via GitHub)" <gi...@apache.org> on 2026/01/20 06:09:51 UTC
[PR] Expose macros module in `airflow.sdk` public API [airflow]
amoghrajesh opened a new pull request, #60808:
URL: https://github.com/apache/airflow/pull/60808
<!--
Thank you for contributing!
Please provide above a brief description of the changes made in this pull request.
Write a good git commit message following this guide: http://chris.beams.io/posts/git-commit/
Please make sure that your code changes are covered with tests.
And in case of new features or big changes remember to adjust the documentation.
Feel free to ping (in general) for the review if you do not see reaction for a few days
(72 Hours is the minimum reaction time you can expect from volunteers) - we sometimes miss notifications.
In case of an existing issue, reference it using one of the following:
* closes: #ISSUE
* related: #ISSUE
-->
---
##### Was generative AI tooling used to co-author this PR?
<!--
If generative AI tooling has been used in the process of authoring this PR, please
change below checkbox to `[X]` followed by the name of the tool, uncomment the "Generated-by".
-->
- [x] No
---
While not a very common practice, users had to import from the internal execution_time path: `from airflow.sdk.execution_time.macros import ds_add` to use macros for utils / custom operator code which required manipulating date/time utilities which macros offered.
I am making a change to expose this better with a short hand: `from airflow.sdk import macros`, this will allow users to
use a simpler public API.
One example use case I came across:
```python
from airflow.sdk import macros, DAG
from datetime import datetime
start_date = macros.ds_add("2024-01-01", -7)
with DAG("my_dag", start_date=start_date):
```
Simple example of usage:
```python
from airflow.sdk.execution_time import macros
macros.ds_add("2026-01-01", 7)
Out[3]: '2026-01-08'
from airflow.sdk import macros
macros.ds_add("2026-01-01", 7)
Out[5]: '2026-01-08'
```
* Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment: `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
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] Expose macros module in `airflow.sdk` public API [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #60808:
URL: https://github.com/apache/airflow/pull/60808#discussion_r2710456214
##########
task-sdk/src/airflow/sdk/__init__.py:
##########
@@ -176,7 +179,13 @@ def __getattr__(name: str):
import importlib
mod = importlib.import_module(module_path, __name__)
- val = getattr(mod, name)
+
+ # try and get the attribute from the module
+ # if it doesn't exist (for modules like macros), return module itself
+ try:
+ val = getattr(mod, name)
+ except AttributeError:
+ val = mod
Review Comment:
The airflow-core version (`airflow-core/src/airflow/__init__.py`) is different - it uses a tuple `(module_path, attr_name, deprecated)` where an empty `attr_name` explicitly signals "return the module itself":
```py
"version": (".version", "", False), # Empty string = return module
"metrics": (".observability.metrics", "", True), # Empty string = return module
```
The check is explicit:
https://github.com/apache/airflow/blob/7267294e622cd897d6585ca55dedde32672acaef/airflow-core/src/airflow/__init__.py#L119-L123
whereas this PR uses a `try/except` fallback which is implicit - it catches `AttributeError` for any reason and assumes "return the module". The airflow-core pattern is intentional, yours is a fallback that could hide bugs.
--
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] Expose macros module in `airflow.sdk` public API [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #60808:
URL: https://github.com/apache/airflow/pull/60808#discussion_r2710456214
##########
task-sdk/src/airflow/sdk/__init__.py:
##########
@@ -176,7 +179,13 @@ def __getattr__(name: str):
import importlib
mod = importlib.import_module(module_path, __name__)
- val = getattr(mod, name)
+
+ # try and get the attribute from the module
+ # if it doesn't exist (for modules like macros), return module itself
+ try:
+ val = getattr(mod, name)
+ except AttributeError:
+ val = mod
Review Comment:
The airflow-core version (`airflow-core/src/airflow/__init__.py) is different - it uses a tuple `(module_path, attr_name, deprecated)` where an empty `attr_name` explicitly signals "return the module itself":
```py
"version": (".version", "", False), # Empty string = return module
"metrics": (".observability.metrics", "", True), # Empty string = return module
```
The check is explicit:
```
if attr_name:
val = getattr(mod, attr_name)
else:
val = mod
```
whereas this PR uses a `try/except` fallback which is implicit - it catches `AttributeError` for any reason and assumes "return the module". The airflow-core pattern is intentional, yours is a fallback that could hide bugs.
##########
task-sdk/src/airflow/sdk/__init__.py:
##########
@@ -176,7 +179,13 @@ def __getattr__(name: str):
import importlib
mod = importlib.import_module(module_path, __name__)
- val = getattr(mod, name)
+
+ # try and get the attribute from the module
+ # if it doesn't exist (for modules like macros), return module itself
+ try:
+ val = getattr(mod, name)
+ except AttributeError:
+ val = mod
Review Comment:
The airflow-core version (`airflow-core/src/airflow/__init__.py) is different - it uses a tuple `(module_path, attr_name, deprecated)` where an empty `attr_name` explicitly signals "return the module itself":
```py
"version": (".version", "", False), # Empty string = return module
"metrics": (".observability.metrics", "", True), # Empty string = return module
```
The check is explicit:
```py
if attr_name:
val = getattr(mod, attr_name)
else:
val = mod
```
whereas this PR uses a `try/except` fallback which is implicit - it catches `AttributeError` for any reason and assumes "return the module". The airflow-core pattern is intentional, yours is a fallback that could hide bugs.
--
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] Expose macros module in `airflow.sdk` public API [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #60808:
URL: https://github.com/apache/airflow/pull/60808#discussion_r2710932890
##########
task-sdk/src/airflow/sdk/__init__.py:
##########
@@ -176,7 +179,13 @@ def __getattr__(name: str):
import importlib
mod = importlib.import_module(module_path, __name__)
- val = getattr(mod, name)
+
+ # try and get the attribute from the module
+ # if it doesn't exist (for modules like macros), return module itself
+ try:
+ val = getattr(mod, name)
+ except AttributeError:
+ val = mod
Review Comment:
Ah ok! Thanks for the clarification @kaxil. Adding to `execution_time/__init__.py` seems to be a better solution
--
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] Expose macros module in `airflow.sdk` public API [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #60808:
URL: https://github.com/apache/airflow/pull/60808#discussion_r2710935264
##########
task-sdk/src/airflow/sdk/__init__.py:
##########
@@ -176,7 +179,13 @@ def __getattr__(name: str):
import importlib
mod = importlib.import_module(module_path, __name__)
- val = getattr(mod, name)
+
+ # try and get the attribute from the module
+ # if it doesn't exist (for modules like macros), return module itself
+ try:
+ val = getattr(mod, name)
+ except AttributeError:
+ val = mod
Review Comment:
@shahar1 handled better in: 7b98b8a64b
--
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] Expose macros module in `airflow.sdk` public API [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on code in PR #60808:
URL: https://github.com/apache/airflow/pull/60808#discussion_r2707031705
##########
task-sdk/src/airflow/sdk/__init__.py:
##########
@@ -176,7 +179,13 @@ def __getattr__(name: str):
import importlib
mod = importlib.import_module(module_path, __name__)
- val = getattr(mod, name)
+
+ # try and get the attribute from the module
+ # if it doesn't exist (for modules like macros), return module itself
+ try:
+ val = getattr(mod, name)
+ except AttributeError:
+ val = mod
Review Comment:
I'm a bit concerned of this workaround, as it might backfire in potential future namespace-naming conflicts. I'd maybe limit it only to the modules that require this:
```suggestion
# try and get the attribute from the module
# if it doesn't exist (for modules like macros), return module itself
try:
val = getattr(mod, name)
except AttributeError:
if name == "macros":
val = mod
else:
raise
```
--
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] Expose macros module in `airflow.sdk` public API [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on code in PR #60808:
URL: https://github.com/apache/airflow/pull/60808#discussion_r2709760595
##########
task-sdk/src/airflow/sdk/__init__.py:
##########
@@ -176,7 +179,13 @@ def __getattr__(name: str):
import importlib
mod = importlib.import_module(module_path, __name__)
- val = getattr(mod, name)
+
+ # try and get the attribute from the module
+ # if it doesn't exist (for modules like macros), return module itself
+ try:
+ val = getattr(mod, name)
+ except AttributeError:
+ val = mod
Review Comment:
> Hmm, I could do that yes, but I do not see why it will backfire, we already use similar patterns for `from airflow import version` or similar
Not a strong objection (maybe I'm extra cautious) - as long as we aware what's going on in the init files I guess that we should be fine.
Fix the failing test and I'll approve.
--
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] Expose macros module in `airflow.sdk` public API [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #60808:
URL: https://github.com/apache/airflow/pull/60808#discussion_r2710449965
##########
task-sdk/src/airflow/sdk/__init__.py:
##########
@@ -176,7 +179,13 @@ def __getattr__(name: str):
import importlib
mod = importlib.import_module(module_path, __name__)
- val = getattr(mod, name)
+
+ # try and get the attribute from the module
+ # if it doesn't exist (for modules like macros), return module itself
+ try:
+ val = getattr(mod, name)
+ except AttributeError:
+ val = mod
Review Comment:
I am with @shahar1 on this. This could mask any typos in `__lazy_imports` which is very likely:
```py
# Hypothetical typo in __lazy_imports:
"DAG_typo": ".definitions.dag",
# Before this PR:
from airflow.sdk import DAG_typo
# AttributeError: module 'airflow.sdk.definitions.dag' has no attribute 'DAG_typo'
# Clear error pointing to the problem
# After this PR:
from airflow.sdk import DAG_typo
# No error - silently returns the module object
DAG_typo(dag_id="test")
# TypeError: 'module' object is not callable
# Confusing error that doesn't point to the real issue
```
A rather simpler solution would be instead of modifying `__getattr__`, just expose macros in `execution_time/__init__.py`:
```py
# task-sdk/src/airflow/sdk/execution_time/__init__.py
from airflow.sdk.execution_time import macros
```
Then change the lazy import to:
```py
"macros": ".execution_time",
```
--
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] Expose macros module in `airflow.sdk` public API [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh merged PR #60808:
URL: https://github.com/apache/airflow/pull/60808
--
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] Expose macros module in `airflow.sdk` public API [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #60808:
URL: https://github.com/apache/airflow/pull/60808#discussion_r2707617133
##########
task-sdk/src/airflow/sdk/__init__.py:
##########
@@ -176,7 +179,13 @@ def __getattr__(name: str):
import importlib
mod = importlib.import_module(module_path, __name__)
- val = getattr(mod, name)
+
+ # try and get the attribute from the module
+ # if it doesn't exist (for modules like macros), return module itself
+ try:
+ val = getattr(mod, name)
+ except AttributeError:
+ val = mod
Review Comment:
Hmm, I could do that yes, but I do not see why it will backfire, we already use similar patterns for `from airflow import version` or similar
--
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] Expose macros module in `airflow.sdk` public API [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on code in PR #60808:
URL: https://github.com/apache/airflow/pull/60808#discussion_r2709760595
##########
task-sdk/src/airflow/sdk/__init__.py:
##########
@@ -176,7 +179,13 @@ def __getattr__(name: str):
import importlib
mod = importlib.import_module(module_path, __name__)
- val = getattr(mod, name)
+
+ # try and get the attribute from the module
+ # if it doesn't exist (for modules like macros), return module itself
+ try:
+ val = getattr(mod, name)
+ except AttributeError:
+ val = mod
Review Comment:
> Hmm, I could do that yes, but I do not see why it will backfire, we already use similar patterns for `from airflow import version` or similar
Not a strong objection (maybe I'm extra cautious) - as long as we are aware what's going on in the init files I guess that we should be fine.
Fix the failing test and I'll approve.
--
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] Expose macros module in `airflow.sdk` public API [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #60808:
URL: https://github.com/apache/airflow/pull/60808#discussion_r2710456214
##########
task-sdk/src/airflow/sdk/__init__.py:
##########
@@ -176,7 +179,13 @@ def __getattr__(name: str):
import importlib
mod = importlib.import_module(module_path, __name__)
- val = getattr(mod, name)
+
+ # try and get the attribute from the module
+ # if it doesn't exist (for modules like macros), return module itself
+ try:
+ val = getattr(mod, name)
+ except AttributeError:
+ val = mod
Review Comment:
The airflow-core version (`airflow-core/src/airflow/__init__.py) is different - it uses a tuple `(module_path, attr_name, deprecated)` where an empty `attr_name` explicitly signals "return the module itself":
```py
"version": (".version", "", False), # Empty string = return module
"metrics": (".observability.metrics", "", True), # Empty string = return module
```
The check is explicit:
https://github.com/apache/airflow/blob/7267294e622cd897d6585ca55dedde32672acaef/airflow-core/src/airflow/__init__.py#L119-L123
whereas this PR uses a `try/except` fallback which is implicit - it catches `AttributeError` for any reason and assumes "return the module". The airflow-core pattern is intentional, yours is a fallback that could hide bugs.
--
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