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/07/08 02:13:50 UTC
[GitHub] [airflow] josh-fell opened a new pull request #16872: Remove/refactor default_args pattern for miscellaneous providers
josh-fell opened a new pull request #16872:
URL: https://github.com/apache/airflow/pull/16872
- Removed or refactored the `default_args` pattern where necessary as requested by @ashb (i.e. removed a separated `default_args` declaration for deference for declaration as part of the `DAG` object)
- Updated example DAG files that access `TaskInstance` objects from context to use `get_current_context()` function
An detailed summary of all changes made as part of this PR can be found below:
| DAG File | Converted `xcom_pull()`? | Other Updates? | Comments |
| ---------| ------------------------ | -------------- | -------- |
| airflow/providers/asana/example_dags/example_asana.py | No | Yes | Removed unneeded `default_args` pattern. |
| airflow/providers/dingding/example_dags/example_dingding.py | No | Yes | Refactored `default_args` pattern. |
| airflow/providers/http/example_dags/example_http.py | No | Yes | Refactored `default_args` pattern. |
| airflow/providers/jdbc/example_dags/example_jdbc_queries.py | No | Yes | Refactored `default_args` pattern. |
| airflow/providers/mysql/example_dags/example_mysql.py | No | Yes | Removed `default_args` pattern. |
| airflow/providers/neo4j/example_dags/example_neo4j.py | No | Yes | Removed `default_args` pattern. |
| airflow/providers/papermill/example_dags/example_papermill.py | No | Yes | Removed `default_args` pattern. |
| airflow/providers/plexus/example_dags/example_plexus.py | No | Yes | Refactored `default_args` pattern. |
| airflow/providers/postgres/example_dags/example_postgres.py | No | Yes | Removed `default_args` pattern. |
| airflow/providers/qubole/example_dags/example_qubole.py | No | Yes | Updated to use `get_current_context()`. |
| airflow/providers/singularity/example_dags/example_singularity.py | No | Yes | Refactored `default_args` pattern. |
| airflow/providers/snowflake/example_dags/example_snowflake.py | No | Yes | Removed `default_args` pattern. |
| airflow/providers/sqlite/example_dags/example_sqlite.py | No | Yes | Removed `default_args` pattern. |
| airflow/providers/tableau/example_dags/example_tableau_refresh_workbook.py | No | Yes | Refactored `default_args` pattern. |
| airflow/providers/telegram/example_dags/example_telegram.py | No | Yes | Removed `default_args` pattern. |
| airflow/providers/yandex/example_dags/example_yandexcloud_dataproc.py | No | Yes | Removed `default_args` pattern. |
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.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] josh-fell commented on a change in pull request #16872: Remove/refactor default_args pattern for miscellaneous providers
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #16872:
URL: https://github.com/apache/airflow/pull/16872#discussion_r666618786
##########
File path: airflow/providers/qubole/example_dags/example_qubole.py
##########
@@ -55,7 +55,7 @@
"""
)
- def compare_result_fn(**kwargs):
+ def compare_result_fn():
Review comment:
Updated to pass in the task instance object and remove the get_current_context() function call.
--
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] josh-fell commented on a change in pull request #16872: Remove/refactor default_args pattern for miscellaneous providers
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #16872:
URL: https://github.com/apache/airflow/pull/16872#discussion_r666619575
##########
File path: airflow/providers/singularity/example_dags/example_singularity.py
##########
@@ -23,19 +23,17 @@
from airflow.providers.singularity.operators.singularity import SingularityOperator
from airflow.utils.dates import days_ago
-default_args = {
- 'owner': 'airflow',
- 'depends_on_past': False,
- 'email': ['airflow@example.com'],
- 'email_on_failure': False,
- 'email_on_retry': False,
- 'retries': 1,
- 'retry_delay': timedelta(minutes=5),
-}
-
with DAG(
'singularity_sample',
- default_args=default_args,
+ default_args={
+ 'owner': 'airflow',
+ 'depends_on_past': False,
+ 'email': ['airflow@example.com'],
+ 'email_on_failure': False,
+ 'email_on_retry': False,
Review comment:
Removed other default_arg args
--
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] josh-fell commented on a change in pull request #16872: Remove/refactor default_args pattern for miscellaneous providers
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #16872:
URL: https://github.com/apache/airflow/pull/16872#discussion_r666618853
##########
File path: airflow/providers/qubole/example_dags/example_qubole.py
##########
@@ -64,7 +64,7 @@ def compare_result_fn(**kwargs):
:return: True if the files are the same, False otherwise.
:rtype: bool
"""
- ti = kwargs['ti']
+ ti = get_current_context()["ti"]
Review comment:
Updated to pass in the task instance object and remove the get_current_context() function call.
--
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] josh-fell commented on a change in pull request #16872: Remove/refactor default_args pattern for miscellaneous providers
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #16872:
URL: https://github.com/apache/airflow/pull/16872#discussion_r666620538
##########
File path: airflow/providers/http/example_dags/example_http.py
##########
@@ -26,17 +26,20 @@
from airflow.providers.http.sensors.http import HttpSensor
from airflow.utils.dates import days_ago
-default_args = {
- 'owner': 'airflow',
- 'depends_on_past': False,
- 'email': ['airflow@example.com'],
- 'email_on_failure': False,
- 'email_on_retry': False,
- 'retries': 1,
- 'retry_delay': timedelta(minutes=5),
-}
-
-dag = DAG('example_http_operator', default_args=default_args, tags=['example'], start_date=days_ago(2))
+dag = DAG(
+ 'example_http_operator',
+ default_args={
+ 'owner': 'airflow',
+ 'depends_on_past': False,
+ 'email': ['airflow@example.com'],
+ 'email_on_failure': False,
+ 'email_on_retry': False,
Review comment:
Updated as suggested.
--
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 change in pull request #16872: Remove/refactor default_args pattern for miscellaneous providers
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #16872:
URL: https://github.com/apache/airflow/pull/16872#discussion_r666005298
##########
File path: airflow/providers/neo4j/example_dags/example_neo4j.py
##########
@@ -23,13 +23,8 @@
from airflow.providers.neo4j.operators.neo4j import Neo4jOperator
from airflow.utils.dates import days_ago
-default_args = {
- 'owner': 'airflow',
-}
-
dag = DAG(
'example_neo4j',
- default_args=default_args,
Review comment:
Owner isn't really _used_ much anywhere anymore, with RBAC it doesn't have much significance anymore anyway.
But either way: if want to show owner we can show it _once_, not on every example dag :)
--
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] josh-fell commented on a change in pull request #16872: Remove/refactor default_args pattern for miscellaneous providers
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #16872:
URL: https://github.com/apache/airflow/pull/16872#discussion_r666621171
##########
File path: airflow/providers/singularity/example_dags/example_singularity.py
##########
@@ -23,19 +23,17 @@
from airflow.providers.singularity.operators.singularity import SingularityOperator
from airflow.utils.dates import days_ago
-default_args = {
- 'owner': 'airflow',
- 'depends_on_past': False,
- 'email': ['airflow@example.com'],
- 'email_on_failure': False,
- 'email_on_retry': False,
- 'retries': 1,
- 'retry_delay': timedelta(minutes=5),
-}
-
with DAG(
'singularity_sample',
- default_args=default_args,
+ default_args={
+ 'owner': 'airflow',
Review comment:
Updated as suggested.
--
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] ephraimbuddy commented on a change in pull request #16872: Remove/refactor default_args pattern for miscellaneous providers
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #16872:
URL: https://github.com/apache/airflow/pull/16872#discussion_r665899179
##########
File path: airflow/providers/neo4j/example_dags/example_neo4j.py
##########
@@ -23,13 +23,8 @@
from airflow.providers.neo4j.operators.neo4j import Neo4jOperator
from airflow.utils.dates import days_ago
-default_args = {
- 'owner': 'airflow',
-}
-
dag = DAG(
'example_neo4j',
- default_args=default_args,
Review comment:
I feel that we shouldn't remove the `owner` argument from the examples. We can however add it to the DAG argument. Like:
```
dag = DAG(...,
default_args={'owner':'airflow'},
...
)
```
This lets people know that such arg is possible
--
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 merged pull request #16872: Remove/refactor default_args pattern for miscellaneous providers
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #16872:
URL: https://github.com/apache/airflow/pull/16872
--
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 change in pull request #16872: Remove/refactor default_args pattern for miscellaneous providers
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #16872:
URL: https://github.com/apache/airflow/pull/16872#discussion_r666005865
##########
File path: airflow/providers/dingding/example_dags/example_dingding.py
##########
@@ -55,12 +50,11 @@ def failure_callback(context):
).execute(context)
-args['on_failure_callback'] = failure_callback
# [END howto_operator_dingding_failure_callback]
with DAG(
dag_id='example_dingding_operator',
- default_args=args,
+ default_args={'owner': 'airflow', 'retries': 3, 'on_failure_callback': failure_callback},
Review comment:
```suggestion
default_args={'retries': 3, 'on_failure_callback': failure_callback},
```
##########
File path: airflow/providers/qubole/example_dags/example_qubole.py
##########
@@ -55,7 +55,7 @@
"""
)
- def compare_result_fn(**kwargs):
+ def compare_result_fn():
Review comment:
Instead of `get_current_context()` (which is something of an escape hatch) we should do this:
```suggestion
def compare_result_fn(ti):
```
##########
File path: airflow/providers/singularity/example_dags/example_singularity.py
##########
@@ -23,19 +23,17 @@
from airflow.providers.singularity.operators.singularity import SingularityOperator
from airflow.utils.dates import days_ago
-default_args = {
- 'owner': 'airflow',
- 'depends_on_past': False,
- 'email': ['airflow@example.com'],
- 'email_on_failure': False,
- 'email_on_retry': False,
- 'retries': 1,
- 'retry_delay': timedelta(minutes=5),
-}
-
with DAG(
'singularity_sample',
- default_args=default_args,
+ default_args={
+ 'owner': 'airflow',
+ 'depends_on_past': False,
Review comment:
```suggestion
```
That is the default
##########
File path: airflow/providers/tableau/example_dags/example_tableau_refresh_workbook.py
##########
@@ -27,17 +27,15 @@
from airflow.providers.tableau.sensors.tableau_job_status import TableauJobStatusSensor
from airflow.utils.dates import days_ago
-DEFAULT_ARGS = {
- 'owner': 'airflow',
- 'depends_on_past': False,
- 'email': ['airflow@example.com'],
- 'email_on_failure': False,
- 'email_on_retry': False,
-}
-
with DAG(
dag_id='example_tableau_refresh_workbook',
- default_args=DEFAULT_ARGS,
+ default_args={
+ 'owner': 'airflow',
+ 'depends_on_past': False,
+ 'email': ['airflow@example.com'],
+ 'email_on_failure': False,
+ 'email_on_retry': False,
+ },
Review comment:
```suggestion
```
Defaults or not actually relevant to example
##########
File path: airflow/providers/singularity/example_dags/example_singularity.py
##########
@@ -23,19 +23,17 @@
from airflow.providers.singularity.operators.singularity import SingularityOperator
from airflow.utils.dates import days_ago
-default_args = {
- 'owner': 'airflow',
- 'depends_on_past': False,
- 'email': ['airflow@example.com'],
- 'email_on_failure': False,
- 'email_on_retry': False,
- 'retries': 1,
- 'retry_delay': timedelta(minutes=5),
-}
-
with DAG(
'singularity_sample',
- default_args=default_args,
+ default_args={
+ 'owner': 'airflow',
+ 'depends_on_past': False,
+ 'email': ['airflow@example.com'],
+ 'email_on_failure': False,
+ 'email_on_retry': False,
Review comment:
```suggestion
```
Not relevant to the example, so lets remove it :)
##########
File path: airflow/providers/singularity/example_dags/example_singularity.py
##########
@@ -23,19 +23,17 @@
from airflow.providers.singularity.operators.singularity import SingularityOperator
from airflow.utils.dates import days_ago
-default_args = {
- 'owner': 'airflow',
- 'depends_on_past': False,
- 'email': ['airflow@example.com'],
- 'email_on_failure': False,
- 'email_on_retry': False,
- 'retries': 1,
- 'retry_delay': timedelta(minutes=5),
-}
-
with DAG(
'singularity_sample',
- default_args=default_args,
+ default_args={
+ 'owner': 'airflow',
Review comment:
```suggestion
```
##########
File path: airflow/providers/qubole/example_dags/example_qubole.py
##########
@@ -64,7 +64,7 @@ def compare_result_fn(**kwargs):
:return: True if the files are the same, False otherwise.
:rtype: bool
"""
- ti = kwargs['ti']
+ ti = get_current_context()["ti"]
Review comment:
```suggestion
```
##########
File path: airflow/providers/http/example_dags/example_http.py
##########
@@ -26,17 +26,20 @@
from airflow.providers.http.sensors.http import HttpSensor
from airflow.utils.dates import days_ago
-default_args = {
- 'owner': 'airflow',
- 'depends_on_past': False,
- 'email': ['airflow@example.com'],
- 'email_on_failure': False,
- 'email_on_retry': False,
- 'retries': 1,
- 'retry_delay': timedelta(minutes=5),
-}
-
-dag = DAG('example_http_operator', default_args=default_args, tags=['example'], start_date=days_ago(2))
+dag = DAG(
+ 'example_http_operator',
+ default_args={
+ 'owner': 'airflow',
+ 'depends_on_past': False,
+ 'email': ['airflow@example.com'],
+ 'email_on_failure': False,
+ 'email_on_retry': False,
Review comment:
```suggestion
```
--
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] josh-fell commented on a change in pull request #16872: Remove/refactor default_args pattern for miscellaneous providers
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #16872:
URL: https://github.com/apache/airflow/pull/16872#discussion_r666622858
##########
File path: airflow/providers/neo4j/example_dags/example_neo4j.py
##########
@@ -23,13 +23,8 @@
from airflow.providers.neo4j.operators.neo4j import Neo4jOperator
from airflow.utils.dates import days_ago
-default_args = {
- 'owner': 'airflow',
-}
-
dag = DAG(
'example_neo4j',
- default_args=default_args,
Review comment:
We do also provide an example of setting the `owner` as part of `default_args` on the Concepts: DAGs documentation (https://airflow.apache.org/docs/apache-airflow/stable/concepts/dags.html). Perhaps that is good enough @ashb?
--
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] ephraimbuddy commented on a change in pull request #16872: Remove/refactor default_args pattern for miscellaneous providers
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #16872:
URL: https://github.com/apache/airflow/pull/16872#discussion_r665899179
##########
File path: airflow/providers/neo4j/example_dags/example_neo4j.py
##########
@@ -23,13 +23,8 @@
from airflow.providers.neo4j.operators.neo4j import Neo4jOperator
from airflow.utils.dates import days_ago
-default_args = {
- 'owner': 'airflow',
-}
-
dag = DAG(
'example_neo4j',
- default_args=default_args,
Review comment:
I feel that we shouldn't remove the `owner` argument from the examples. We can however add it to the DAG argument. Like:
```
dag = DAG(...,
default_args={'owner':'airflow'},
...
)
```
This lets people know that such arg is possible
--
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] josh-fell commented on a change in pull request #16872: Remove/refactor default_args pattern for miscellaneous providers
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #16872:
URL: https://github.com/apache/airflow/pull/16872#discussion_r666621228
##########
File path: airflow/providers/singularity/example_dags/example_singularity.py
##########
@@ -23,19 +23,17 @@
from airflow.providers.singularity.operators.singularity import SingularityOperator
from airflow.utils.dates import days_ago
-default_args = {
- 'owner': 'airflow',
- 'depends_on_past': False,
- 'email': ['airflow@example.com'],
- 'email_on_failure': False,
- 'email_on_retry': False,
- 'retries': 1,
- 'retry_delay': timedelta(minutes=5),
-}
-
with DAG(
'singularity_sample',
- default_args=default_args,
+ default_args={
+ 'owner': 'airflow',
+ 'depends_on_past': False,
Review comment:
Updated as suggested.
--
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