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/08/18 13:20:06 UTC
[GitHub] [airflow] ashb opened a new pull request, #25795: Let timetables control generated run_ids.
ashb opened a new pull request, #25795:
URL: https://github.com/apache/airflow/pull/25795
Rather than a cluster policy level setting I have decided to add this to the Timetable as that felt like a more natural fit to me (Timetables are responsible for saying when a DagRun should be created, extending that to also make it say _what_ the run should be called felt like a natural etension to me.)
This isn't _quite_ as easy to control as the OP asked for (where you could just subclass `DAG` and add a method; timetables have to be "registered" via a plugin, but that can be in-tree).
Closes #21923
<!--
Thank you for contributing! 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 committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ 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 changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+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 a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/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
[GitHub] [airflow] ashb merged pull request #25795: Let timetables control generated run_ids.
Posted by GitBox <gi...@apache.org>.
ashb merged PR #25795:
URL: https://github.com/apache/airflow/pull/25795
--
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] norm commented on a diff in pull request #25795: Let timetables control generated run_ids.
Posted by GitBox <gi...@apache.org>.
norm commented on code in PR #25795:
URL: https://github.com/apache/airflow/pull/25795#discussion_r949986401
##########
airflow/timetables/base.py:
##########
@@ -200,3 +203,15 @@ def next_dagrun_info(
a DagRunInfo object when asked at another time.
"""
raise NotImplementedError()
+
+ def generate_run_id(
+ self,
+ *,
+ run_type: "DagRunType",
+ logical_date: DateTime,
+ data_interval: Optional[DataInterval],
+ **extra,
+ ) -> str:
+ from airflow.models.dagrun import DagRun
+
+ return DagRun.generate_run_id(run_type, logical_date)
Review Comment:
At first glance, I found it unexpected that the responsibility has moved here, but the code still lives on `DagRun`. Not a request for change, just an observation.
--
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 #25795: Let timetables control generated run_ids.
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25795:
URL: https://github.com/apache/airflow/pull/25795#discussion_r950001227
##########
airflow/timetables/base.py:
##########
@@ -200,3 +203,15 @@ def next_dagrun_info(
a DagRunInfo object when asked at another time.
"""
raise NotImplementedError()
+
+ def generate_run_id(
+ self,
+ *,
+ run_type: "DagRunType",
+ logical_date: DateTime,
+ data_interval: Optional[DataInterval],
+ **extra,
+ ) -> str:
+ from airflow.models.dagrun import DagRun
+
+ return DagRun.generate_run_id(run_type, logical_date)
Review Comment:
Same, this should probably become a free function.
--
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 #25795: Let timetables control generated run_ids.
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25795:
URL: https://github.com/apache/airflow/pull/25795#discussion_r950002607
##########
airflow/api/common/trigger_dag.py:
##########
@@ -68,7 +68,9 @@ def _trigger_dag(
f"[{min_dag_start_date.isoformat()}] from DAG's default_args"
)
- run_id = run_id or DagRun.generate_run_id(DagRunType.MANUAL, execution_date)
+ run_id = run_id or dag.timetable.generate_run_id(
+ run_type=DagRunType.MANUAL, logical_date=execution_date, data_interval=None
Review Comment:
We always need a data interval anyway, so the `data_interval=None` case can be avoided by moving the data interval logic before this line.
--
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 #25795: Let timetables control generated run_ids.
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #25795:
URL: https://github.com/apache/airflow/pull/25795#discussion_r950031806
##########
airflow/timetables/base.py:
##########
@@ -200,3 +203,15 @@ def next_dagrun_info(
a DagRunInfo object when asked at another time.
"""
raise NotImplementedError()
+
+ def generate_run_id(
+ self,
+ *,
+ run_type: "DagRunType",
+ logical_date: DateTime,
+ data_interval: Optional[DataInterval],
+ **extra,
+ ) -> str:
+ from airflow.models.dagrun import DagRun
+
+ return DagRun.generate_run_id(run_type, logical_date)
Review Comment:
I'll move this in to DagRunType (enum) class that we already have.
--
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 pull request #25795: Let timetables control generated run_ids.
Posted by GitBox <gi...@apache.org>.
ashb commented on PR #25795:
URL: https://github.com/apache/airflow/pull/25795#issuecomment-1220502033
@norm @uranusjr Please take another look
--
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 pull request #25795: Let timetables control generated run_ids.
Posted by GitBox <gi...@apache.org>.
ashb commented on PR #25795:
URL: https://github.com/apache/airflow/pull/25795#issuecomment-1219486210
@KevinJiao @dukarc Please take a look. Not quite the way you asked for it, but the ability to customized run_id should be back with 2.4.
--
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 #25795: Let timetables control generated run_ids.
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #25795:
URL: https://github.com/apache/airflow/pull/25795#discussion_r950023018
##########
airflow/timetables/base.py:
##########
@@ -200,3 +203,15 @@ def next_dagrun_info(
a DagRunInfo object when asked at another time.
"""
raise NotImplementedError()
+
+ def generate_run_id(
+ self,
+ *,
+ run_type: "DagRunType",
+ logical_date: DateTime,
+ data_interval: Optional[DataInterval],
+ **extra,
+ ) -> str:
+ from airflow.models.dagrun import DagRun
+
+ return DagRun.generate_run_id(run_type, logical_date)
Review Comment:
Yeah fair comment.
--
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