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 2022/04/08 14:24:44 UTC
[GitHub] [airflow] ashb opened a new pull request, #22862: Add XComArg to lazy-imported list of Airflow module
ashb opened a new pull request, #22862:
URL: https://github.com/apache/airflow/pull/22862
In writing the docs for Dynamic Task Mapping (AIP-42) I noticed that
there are some cases where users need to use XComArg directly, and it
didn't feel right to make the import things from `airflow.models`.
And I've now refactored the lazy import to be "data-driven" as three
blocks of almost identical code was my limit.
Example dag from the docs (future PR) that inspired this change:
```python
from datetime import datetime
from airflow import DAG
from airflow.decorators import task
from airflow.models.xcom_arg import XComArg # <--- this I wasn't happy about
from airflow.providers.amazon.aws.hooks.s3 import S3Hook
from airflow.providers.amazon.aws.operators.s3 import S3ListOperator
with DAG(dag_id="mapped_s3", start_date=datetime(2020,4,7)) as dag:
files = S3ListOperator(
task_id="get_input",
bucket="example-bucket",
prefix='incoming/provider_a/{{ data_interval_start.strftime("%Y-%m-%d") }}',
)
@task
def count_lines(aws_conn_id, bucket, file):
hook = S3Hook(aws_conn_id=aws_conn_id)
return len(hook.read_key(file, bucket).splitlines())
@task
def total(lines):
return sum(lines)
counts = count_lines.partial(aws_conn_id="aws_default", bucket=files.bucket).expand(file=XComArg(files))
total(lines=counts)
```
--
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] ashb commented on a diff in pull request #22862: Add XComArg to lazy-imported list of Airflow module
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #22862:
URL: https://github.com/apache/airflow/pull/22862#discussion_r846187983
##########
airflow/__init__.py:
##########
@@ -37,7 +37,7 @@
__version__ = version.version
-__all__ = ['__version__', 'login', 'DAG', 'PY36', 'PY37', 'PY38', 'PY39', 'PY310']
+__all__ = ['__version__', 'login', 'DAG', 'PY36', 'PY37', 'PY38', 'PY39', 'PY310', 'XComArg']
Review Comment:
Oh Sorry, I See. Dunno. 😁
--
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] uranusjr commented on a diff in pull request #22862: Add XComArg to lazy-imported list of Airflow module
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #22862:
URL: https://github.com/apache/airflow/pull/22862#discussion_r846213568
##########
airflow/__init__.py:
##########
@@ -37,7 +37,7 @@
__version__ = version.version
-__all__ = ['__version__', 'login', 'DAG', 'PY36', 'PY37', 'PY38', 'PY39', 'PY310']
+__all__ = ['__version__', 'login', 'DAG', 'PY36', 'PY37', 'PY38', 'PY39', 'PY310', 'XComArg']
Review Comment:
Given that `DAG` is, perhaps? (Also why is `AirflowException` not in this list?)
--
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] jedcunningham commented on a diff in pull request #22862: Add XComArg to lazy-imported list of Airflow module
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #22862:
URL: https://github.com/apache/airflow/pull/22862#discussion_r846172883
##########
airflow/__init__.py:
##########
@@ -54,18 +54,30 @@
PY39 = sys.version_info >= (3, 9)
PY310 = sys.version_info >= (3, 10)
+# Things to lazy import in form 'name': 'path.to.module'
+__lazy_imports = {
+ 'DAG': 'airflow.models.dag',
+ 'XComArg': 'airflow.models.xcom_arg',
+ 'AirflowException': 'airflow.exceptions',
+}
+
def __getattr__(name):
# PEP-562: Lazy loaded attributes on python modules
- if name == "DAG":
- from airflow.models.dag import DAG
+ path = __lazy_imports.get(name)
+ if path:
+ import operator
Review Comment:
```suggestion
if not path:
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
import operator
```
nit
##########
airflow/__init__.py:
##########
@@ -54,18 +54,30 @@
PY39 = sys.version_info >= (3, 9)
PY310 = sys.version_info >= (3, 10)
+# Things to lazy import in form 'name': 'path.to.module'
+__lazy_imports = {
+ 'DAG': 'airflow.models.dag',
+ 'XComArg': 'airflow.models.xcom_arg',
+ 'AirflowException': 'airflow.exceptions',
+}
+
def __getattr__(name):
# PEP-562: Lazy loaded attributes on python modules
- if name == "DAG":
- from airflow.models.dag import DAG
+ path = __lazy_imports.get(name)
+ if path:
+ import operator
- return DAG
- if name == "AirflowException":
- from airflow.exceptions import AirflowException
+ # Strip of the "airflow." prefix because of how `__import__` works (it always returns the top level
+ # module)
+ without_prefix = path.split('.', 1)[-1]
Review Comment:
```suggestion
# Strip of the "airflow." prefix because of how `__import__` works (it always returns the top level
# module)
without_prefix = path.split('.', 1)[-1]
```
nit cont.
##########
airflow/__init__.py:
##########
@@ -54,18 +54,30 @@
PY39 = sys.version_info >= (3, 9)
PY310 = sys.version_info >= (3, 10)
+# Things to lazy import in form 'name': 'path.to.module'
+__lazy_imports = {
+ 'DAG': 'airflow.models.dag',
+ 'XComArg': 'airflow.models.xcom_arg',
+ 'AirflowException': 'airflow.exceptions',
+}
+
def __getattr__(name):
# PEP-562: Lazy loaded attributes on python modules
- if name == "DAG":
- from airflow.models.dag import DAG
+ path = __lazy_imports.get(name)
+ if path:
+ import operator
- return DAG
- if name == "AirflowException":
- from airflow.exceptions import AirflowException
+ # Strip of the "airflow." prefix because of how `__import__` works (it always returns the top level
+ # module)
+ without_prefix = path.split('.', 1)[-1]
- return AirflowException
- raise AttributeError(f"module {__name__} has no attribute {name}")
+ getter = operator.attrgetter(f'{without_prefix}.{name}')
+ val = getter(__import__(path))
+ # Store for next time
+ globals()[name] = val
+ return val
+ raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
Review Comment:
```suggestion
getter = operator.attrgetter(f'{without_prefix}.{name}')
val = getter(__import__(path))
# Store for next time
globals()[name] = val
return val
```
nit cont.
--
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] ferruzzi commented on a diff in pull request #22862: Add XComArg to lazy-imported list of Airflow module
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on code in PR #22862:
URL: https://github.com/apache/airflow/pull/22862#discussion_r846285920
##########
airflow/__init__.py:
##########
@@ -54,18 +54,31 @@
PY39 = sys.version_info >= (3, 9)
PY310 = sys.version_info >= (3, 10)
+# Things to lazy import in form 'name': 'path.to.module'
+__lazy_imports = {
+ 'DAG': 'airflow.models.dag',
+ 'XComArg': 'airflow.models.xcom_arg',
+ 'AirflowException': 'airflow.exceptions',
+}
+
def __getattr__(name):
# PEP-562: Lazy loaded attributes on python modules
- if name == "DAG":
- from airflow.models.dag import DAG
+ path = __lazy_imports.get(name)
+ if not path:
+ raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
+
+ import operator
- return DAG
- if name == "AirflowException":
- from airflow.exceptions import AirflowException
+ # Strip of the "airflow." prefix because of how `__import__` works (it always returns the top level
Review Comment:
```suggestion
# Strip off the "airflow." prefix because of how `__import__` works (it always returns the top level
```
--
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] ashb commented on a diff in pull request #22862: Add XComArg to lazy-imported list of Airflow module
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #22862:
URL: https://github.com/apache/airflow/pull/22862#discussion_r846287606
##########
airflow/__init__.py:
##########
@@ -37,7 +37,7 @@
__version__ = version.version
-__all__ = ['__version__', 'login', 'DAG', 'PY36', 'PY37', 'PY38', 'PY39', 'PY310']
+__all__ = ['__version__', 'login', 'DAG', 'PY36', 'PY37', 'PY38', 'PY39', 'PY310', 'XComArg']
Review Comment:
No idea why AirflowException wasn't 🤷🏻 at this point though
--
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] ashb merged pull request #22862: Add XComArg to lazy-imported list of Airflow module
Posted by GitBox <gi...@apache.org>.
ashb merged PR #22862:
URL: https://github.com/apache/airflow/pull/22862
--
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] github-actions[bot] commented on pull request #22862: Add XComArg to lazy-imported list of Airflow module
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22862:
URL: https://github.com/apache/airflow/pull/22862#issuecomment-1092936850
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
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] jedcunningham commented on a diff in pull request #22862: Add XComArg to lazy-imported list of Airflow module
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #22862:
URL: https://github.com/apache/airflow/pull/22862#discussion_r846183402
##########
airflow/__init__.py:
##########
@@ -37,7 +37,7 @@
__version__ = version.version
-__all__ = ['__version__', 'login', 'DAG', 'PY36', 'PY37', 'PY38', 'PY39', 'PY310']
+__all__ = ['__version__', 'login', 'DAG', 'PY36', 'PY37', 'PY38', 'PY39', 'PY310', 'XComArg']
Review Comment:
Is this worth adding to `__all__`?
--
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] ashb commented on a diff in pull request #22862: Add XComArg to lazy-imported list of Airflow module
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #22862:
URL: https://github.com/apache/airflow/pull/22862#discussion_r846184594
##########
airflow/__init__.py:
##########
@@ -37,7 +37,7 @@
__version__ = version.version
-__all__ = ['__version__', 'login', 'DAG', 'PY36', 'PY37', 'PY38', 'PY39', 'PY310']
+__all__ = ['__version__', 'login', 'DAG', 'PY36', 'PY37', 'PY38', 'PY39', 'PY310', 'XComArg']
Review Comment:
Yes, I meant to.
--
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