You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/12/29 01:12:12 UTC
[GitHub] [airflow] rustikk opened a new issue #20545: built in macros (macros.random, macros.time) import functions instead of modules
rustikk opened a new issue #20545:
URL: https://github.com/apache/airflow/issues/20545
### Apache Airflow version
2.2.3 (latest released)
### What happened
My gut says that the way forward is to change the macros object so that it only exposes modules:
- datetime
- time
- uuid
- random
... and then leave it to the user to decide which functions on those modules they want to call. I'm not confident enough to make that change. If instead we want to change the docs to match the actual functionality, I can submit a PR for that.
### What you expected to happen
When using either the bultin macros time or random they don't call datetime.time or random they instead call the builtin module time or for random returns a function instead of the module.
### How to reproduce
```
import datetime as dt
import time
from uuid import uuid4
from textwrap import dedent
from airflow.models import DAG
from airflow.operators.python import PythonOperator
from dateutil.parser import parse as dateutil_parse
"""
According to the docs:
macros.datetime - datetime.datetime
macros.timedelta - datetime.timedelta
macros.datetutil - dateutil package
macros.time - datetime.time
macros.uuid - python standard lib uuid
macros.random - python standard lib random
According to the code:
macros.datetime - datetime.datetime
macros.timedelta - datetime.timedelta
macros.datetutil - dateutil package
macros.time - python standard lib time <--- differs
macros.uuid - python standard lib uuid
macros.random - random.random <--- differs
"""
def date_time(datetime_obj):
compare_obj = dt.datetime(2021, 12, 12, 8, 32, 23)
assert datetime_obj == compare_obj
def time_delta(timedelta_obj):
compare_obj = dt.timedelta(days=3, hours=4)
assert timedelta_obj == compare_obj
def date_util(dateutil_obj):
compare_obj = dateutil_parse("Thu Sep 26 10:36:28 2019")
assert dateutil_obj == compare_obj
def time_tester(time_obj):
# note that datetime.time.time() gives an AttributeError
# time.time() on the other hand, returns a float
# this works because macro.time isn't 'datetime.time', like the docs say
# it's just 'time'
compare_obj = time.time()
print(time_obj)
print(compare_obj)
# the macro might have captured a slightly differnt time than the task,
# but they're not going to be more than 10s apart
assert abs(time_obj - compare_obj) < 10
def uuid_tester(uuid_obj):
compare_obj = uuid4()
assert len(str(uuid_obj)) == len(str(compare_obj))
def random_tester(random_float):
# note that 'random.random' is a function that returns a float
# while 'random' is a module (and isn't callable)
# the macro was 'macros.random()' and here we have a float:
assert -0.1 < random_float < 100.1
# so the docs are wrong here too
# macros.random actually returns a function, not the random module
def show_docs(attr):
print(attr.__doc__)
with DAG(
dag_id="builtin_macros_with_airflow_specials",
schedule_interval=None,
start_date=dt.datetime(1970, 1, 1),
render_template_as_native_obj=True, # render templates using Jinja NativeEnvironment
tags=["core"],
) as dag:
test_functions = {
"datetime": (date_time, "{{ macros.datetime(2021, 12, 12, 8, 32, 23) }}"),
"timedelta": (time_delta, "{{ macros.timedelta(days=3, hours=4) }}"),
"dateutil": (
date_util,
"{{ macros.dateutil.parser.parse('Thu Sep 26 10:36:28 2019') }}",
),
"time": (time_tester, "{{ macros.time.time() }}"),
"uuid": (uuid_tester, "{{ macros.uuid.uuid4() }}"),
"random": (
random_tester,
"{{ 100 * macros.random() }}",
),
}
for name, (func, template) in test_functions.items():
(
PythonOperator(
task_id=f"showdoc_{name}",
python_callable=show_docs,
op_args=[f"{{{{ macros.{name} }}}}"],
)
>> PythonOperator(
task_id=f"test_{name}", python_callable=func, op_args=[template]
)
)
```
### Operating System
Docker (debian:buster)
### Versions of Apache Airflow Providers
2.2.2, and 2.2.3
### Deployment
Other
### Deployment details
Astro CLI with sequential executor
### Anything else
Rather than changing the docs to describe what the code actually does would it be better to make the code behave in a way that is more consistent (e.g. toplevel modules only)?
### Are you willing to submit PR?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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
[GitHub] [airflow] potiuk commented on issue #20545: built in macros (macros.random, macros.time) need documentation change
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #20545:
URL: https://github.com/apache/airflow/issues/20545#issuecomment-1002787051
Feel free!
--
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
[GitHub] [airflow] rustikk commented on issue #20545: built in macros (macros.random, macros.time) import functions instead of modules
Posted by GitBox <gi...@apache.org>.
rustikk commented on issue #20545:
URL: https://github.com/apache/airflow/issues/20545#issuecomment-1002786210
If it's ok I'd like to update the docs on this issue.
--
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
[GitHub] [airflow] kaxil closed issue #20545: built in macros (macros.random, macros.time) need documentation change
Posted by GitBox <gi...@apache.org>.
kaxil closed issue #20545:
URL: https://github.com/apache/airflow/issues/20545
--
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
[GitHub] [airflow] potiuk commented on issue #20545: built in macros (macros.random, macros.time) import functions instead of modules
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #20545:
URL: https://github.com/apache/airflow/issues/20545#issuecomment-1002603446
I think that would be a heavily breaking change. I think we should update the docs instead.
--
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