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 2020/12/24 04:00:25 UTC
[GitHub] [airflow] jmelot opened a new pull request #13297: Use DAG context manager in examples (#12894)
jmelot opened a new pull request #13297:
URL: https://github.com/apache/airflow/pull/13297
closes: #12894
Please let me know if I need to make any changes!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #13297: Use DAG context manager in examples (#12894)
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #13297:
URL: https://github.com/apache/airflow/pull/13297#discussion_r559051392
##########
File path: airflow/example_dags/tutorial_etl_dag.py
##########
@@ -99,32 +99,32 @@ def load(**kwargs):
python_callable=extract,
)
extract_task.doc_md = """\
Review comment:
I guess it might require `textwrap.dedent` to be added.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jmelot commented on a change in pull request #13297: Use DAG context manager in examples (#12894)
Posted by GitBox <gi...@apache.org>.
jmelot commented on a change in pull request #13297:
URL: https://github.com/apache/airflow/pull/13297#discussion_r560598808
##########
File path: airflow/example_dags/example_bash_operator.py
##########
@@ -29,47 +29,43 @@
'owner': 'airflow',
}
-dag = DAG(
+with DAG(
dag_id='example_bash_operator',
default_args=args,
schedule_interval='0 0 * * *',
start_date=days_ago(2),
dagrun_timeout=timedelta(minutes=60),
tags=['example', 'example2'],
params={"example_key": "example_value"},
-)
+) as dag:
-run_this_last = DummyOperator(
- task_id='run_this_last',
- dag=dag,
-)
+ run_this_last = DummyOperator(
+ task_id='run_this_last',
+ )
+
+ # [START howto_operator_bash]
+ run_this = BashOperator(
+ task_id='run_after_loop',
+ bash_command='echo 1',
+ )
+ # [END howto_operator_bash]
-# [START howto_operator_bash]
-run_this = BashOperator(
- task_id='run_after_loop',
- bash_command='echo 1',
- dag=dag,
-)
-# [END howto_operator_bash]
+ run_this >> run_this_last
-run_this >> run_this_last
+ for i in range(3):
+ task = BashOperator(
+ task_id='runme_' + str(i),
+ bash_command='echo "{{ task_instance_key_str }}" && sleep 1',
+ )
+ task >> run_this
-for i in range(3):
- task = BashOperator(
- task_id='runme_' + str(i),
- bash_command='echo "{{ task_instance_key_str }}" && sleep 1',
- dag=dag,
+ # [START howto_operator_bash_template]
+ also_run_this = BashOperator(
+ task_id='also_run_this',
+ bash_command='echo "run_id={{ run_id }} | dag_run={{ dag_run }}"',
)
- task >> run_this
-
-# [START howto_operator_bash_template]
-also_run_this = BashOperator(
- task_id='also_run_this',
- bash_command='echo "run_id={{ run_id }} | dag_run={{ dag_run }}"',
- dag=dag,
-)
-# [END howto_operator_bash_template]
-also_run_this >> run_this_last
+ # [END howto_operator_bash_template]
+ also_run_this >> run_this_last
-if __name__ == "__main__":
- dag.cli()
+ if __name__ == "__main__":
+ dag.cli()
Review comment:
Done, thanks for pointing that out!
##########
File path: airflow/example_dags/tutorial_etl_dag.py
##########
@@ -99,32 +99,32 @@ def load(**kwargs):
python_callable=extract,
)
extract_task.doc_md = """\
Review comment:
Done!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13297: Use DAG context manager in examples (#12894)
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13297:
URL: https://github.com/apache/airflow/pull/13297#discussion_r558303297
##########
File path: airflow/example_dags/example_bash_operator.py
##########
@@ -29,47 +29,43 @@
'owner': 'airflow',
}
-dag = DAG(
+with DAG(
dag_id='example_bash_operator',
default_args=args,
schedule_interval='0 0 * * *',
start_date=days_ago(2),
dagrun_timeout=timedelta(minutes=60),
tags=['example', 'example2'],
params={"example_key": "example_value"},
-)
+) as dag:
-run_this_last = DummyOperator(
- task_id='run_this_last',
- dag=dag,
-)
+ run_this_last = DummyOperator(
+ task_id='run_this_last',
+ )
+
+ # [START howto_operator_bash]
+ run_this = BashOperator(
+ task_id='run_after_loop',
+ bash_command='echo 1',
+ )
+ # [END howto_operator_bash]
-# [START howto_operator_bash]
-run_this = BashOperator(
- task_id='run_after_loop',
- bash_command='echo 1',
- dag=dag,
-)
-# [END howto_operator_bash]
+ run_this >> run_this_last
-run_this >> run_this_last
+ for i in range(3):
+ task = BashOperator(
+ task_id='runme_' + str(i),
+ bash_command='echo "{{ task_instance_key_str }}" && sleep 1',
+ )
+ task >> run_this
-for i in range(3):
- task = BashOperator(
- task_id='runme_' + str(i),
- bash_command='echo "{{ task_instance_key_str }}" && sleep 1',
- dag=dag,
+ # [START howto_operator_bash_template]
+ also_run_this = BashOperator(
+ task_id='also_run_this',
+ bash_command='echo "run_id={{ run_id }} | dag_run={{ dag_run }}"',
)
- task >> run_this
-
-# [START howto_operator_bash_template]
-also_run_this = BashOperator(
- task_id='also_run_this',
- bash_command='echo "run_id={{ run_id }} | dag_run={{ dag_run }}"',
- dag=dag,
-)
-# [END howto_operator_bash_template]
-also_run_this >> run_this_last
+ # [END howto_operator_bash_template]
+ also_run_this >> run_this_last
-if __name__ == "__main__":
- dag.cli()
+ if __name__ == "__main__":
+ dag.cli()
Review comment:
```suggestion
if __name__ == "__main__":
dag.cli()
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #13297: Use DAG context manager in examples (#12894)
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #13297:
URL: https://github.com/apache/airflow/pull/13297#discussion_r549239940
##########
File path: airflow/example_dags/example_bash_operator.py
##########
@@ -29,47 +29,43 @@
'owner': 'airflow',
}
-dag = DAG(
+with DAG(
dag_id='example_bash_operator',
default_args=args,
schedule_interval='0 0 * * *',
start_date=days_ago(2),
dagrun_timeout=timedelta(minutes=60),
tags=['example', 'example2'],
params={"example_key": "example_value"},
-)
+) as dag:
-run_this_last = DummyOperator(
- task_id='run_this_last',
- dag=dag,
-)
+ run_this_last = DummyOperator(
+ task_id='run_this_last',
+ )
+
+ # [START howto_operator_bash]
+ run_this = BashOperator(
Review comment:
Could you please update the documentation and remove the added indent? You can do it with " :dedent:" option. For example, see: https://github.com/apache/airflow/blob/master/docs/apache-airflow-providers-google/operators/cloud/bigquery.rst
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #13297: Use DAG context manager in examples (#12894)
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #13297:
URL: https://github.com/apache/airflow/pull/13297#issuecomment-762609525
what is the status of this change? Is it ready for review?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jmelot commented on a change in pull request #13297: Use DAG context manager in examples (#12894)
Posted by GitBox <gi...@apache.org>.
jmelot commented on a change in pull request #13297:
URL: https://github.com/apache/airflow/pull/13297#discussion_r550488423
##########
File path: airflow/example_dags/example_bash_operator.py
##########
@@ -29,47 +29,43 @@
'owner': 'airflow',
}
-dag = DAG(
+with DAG(
dag_id='example_bash_operator',
default_args=args,
schedule_interval='0 0 * * *',
start_date=days_ago(2),
dagrun_timeout=timedelta(minutes=60),
tags=['example', 'example2'],
params={"example_key": "example_value"},
-)
+) as dag:
-run_this_last = DummyOperator(
- task_id='run_this_last',
- dag=dag,
-)
+ run_this_last = DummyOperator(
+ task_id='run_this_last',
+ )
+
+ # [START howto_operator_bash]
+ run_this = BashOperator(
Review comment:
Thanks for the example, done! I went ahead and made this change in some other examples I hadn't modified as well (like in kubernetes.rst) that had similar indent issues.
While I was looking at this, I noticed that further down in the bash.rst (and probably other documentation files), the context manager is not used in code blocks that were written directly into the documentation, like in the last example [here](https://github.com/apache/airflow/blob/96150401c1344e818d91084da057824afd630781/docs/apache-airflow/howto/operator/bash.rst). I could update the documentation wherever this occurs if you thought that made sense (wasn't sure if it would be too far out of the scope of the original issue, 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13297: Use DAG context manager in examples (#12894)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13297:
URL: https://github.com/apache/airflow/pull/13297#issuecomment-750741119
[The Workflow run](https://github.com/apache/airflow/actions/runs/441694802) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj merged pull request #13297: Use DAG context manager in examples (#12894)
Posted by GitBox <gi...@apache.org>.
mik-laj merged pull request #13297:
URL: https://github.com/apache/airflow/pull/13297
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jmelot commented on pull request #13297: Use DAG context manager in examples (#12894)
Posted by GitBox <gi...@apache.org>.
jmelot commented on pull request #13297:
URL: https://github.com/apache/airflow/pull/13297#issuecomment-763251009
@mik-laj Yes, I think so. I've added the dedent to the docstring. Thanks for your patience in pointing this out!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org