You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "sunank200 (via GitHub)" <gi...@apache.org> on 2025/02/06 10:30:10 UTC
[PR] Change default logical date to null when manual trigger from UI [airflow]
sunank200 opened a new pull request, #46512:
URL: https://github.com/apache/airflow/pull/46512
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
This PR changes the default logical date to null when a manual trigger from UI. This is part of the AIP-83 amendment.
reference: [https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=337676722#Option2clarificationdocWIP-Resolutionofquestions](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=337676722#Option2clarificationdocWIP-Resolutionofquestions)
closes: #46455
<!--
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/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1953916801
##########
airflow/cli/commands/remote_commands/task_command.py:
##########
@@ -129,28 +127,32 @@ def _get_dag_run(
f"of {logical_date_or_run_id!r} not found"
)
- dag_run_logical_date = pendulum.instance(logical_date or timezone.utcnow())
+ dag_run_logical_date = timezone.coerce_datetime(logical_date) if logical_date else None
Review Comment:
changed it.
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1948853445
##########
airflow/models/dag.py:
##########
@@ -1613,6 +1614,7 @@ def test(
"""
Execute one single DagRun for a given DAG and logical date.
+ :param run_after: DagRun to execute after this DagRun
Review Comment:
I feel this one is inaccurate?
##########
airflow/api/common/trigger_dag.py:
##########
@@ -64,27 +64,31 @@ def _trigger_dag(
if dag is None or dag_id not in dag_bag.dags:
raise DagNotFound(f"Dag id {dag_id} not found")
-
- logical_date = logical_date or timezone.utcnow()
-
- if not timezone.is_localized(logical_date):
- raise ValueError("The logical date should be localized")
-
- if replace_microseconds:
- logical_date = logical_date.replace(microsecond=0)
-
- if dag.default_args and "start_date" in dag.default_args:
- min_dag_start_date = dag.default_args["start_date"]
- if min_dag_start_date and logical_date < min_dag_start_date:
- raise ValueError(
- f"Logical date [{logical_date.isoformat()}] should be >= start_date "
- f"[{min_dag_start_date.isoformat()}] from DAG's default_args"
- )
- coerced_logical_date = timezone.coerce_datetime(logical_date)
-
- data_interval = dag.timetable.infer_manual_data_interval(run_after=coerced_logical_date)
+ if logical_date:
+ if not timezone.is_localized(logical_date):
+ raise ValueError("The logical date should be localized")
+
+ if replace_microseconds:
+ logical_date = logical_date.replace(microsecond=0)
+
+ if dag.default_args and "start_date" in dag.default_args:
+ min_dag_start_date = dag.default_args["start_date"]
+ if min_dag_start_date and logical_date < min_dag_start_date:
+ raise ValueError(
+ f"Logical date [{logical_date.isoformat()}] should be >= start_date "
+ f"[{min_dag_start_date.isoformat()}] from DAG's default_args"
+ )
+ run_after = timezone.coerce_datetime(logical_date)
+ data_interval = dag.timetable.infer_manual_data_interval(run_after=run_after)
+ else:
+ data_interval = None
+ run_after = timezone.coerce_datetime(timezone.utcnow())
Review Comment:
```suggestion
data_interval = dag.timetable.infer_manual_data_interval(run_after=run_after)
run_after = timezone.coerce_datetime(logical_date)
else:
data_interval = None
run_after = timezone.coerce_datetime(timezone.utcnow())
```
nit: reorder itt to keep the same reading order
##########
airflow/api_connexion/schemas/dag_run_schema.py:
##########
@@ -78,7 +78,7 @@ class Meta:
@pre_load
def autogenerate(self, data, **kwargs):
- """Auto generate run_id and logical_date if they are not provided."""
+ """Auto generate run_id, logical_date if they are not provided."""
Review Comment:
why do we change it here? just rewording, or the content should be changed as well
##########
airflow/models/dag.py:
##########
@@ -1782,6 +1789,10 @@ def create_dagrun(
:meta private:
"""
logical_date = timezone.coerce_datetime(logical_date)
+ # For manual runs where logical_date is None, ensure no data_interval is set.
+ if logical_date is None:
+ if data_interval is not None:
+ raise ValueError("data_interval must be None when logical_date is None")
Review Comment:
```suggestion
if logical_date is None and data_interval is not None:
raise ValueError("data_interval must be None when logical_date is None")
```
##########
airflow/models/dag.py:
##########
@@ -1602,6 +1602,7 @@ def cli(self):
@provide_session
def test(
self,
+ run_after: datetime | None = None,
Review Comment:
I thought run_after could not be None? https://github.com/apache/airflow/pull/46398#discussion_r1944139126
##########
airflow/models/dagrun.py:
##########
@@ -253,12 +253,15 @@ def __init__(
dag_version: DagVersion | None = None,
bundle_version: str | None = None,
):
+ # For manual runs where logical_date is None, ensure no data_interval is set.
+ if logical_date is None:
+ if data_interval is not None:
+ raise ValueError("data_interval must be None if logical_date is None")
Review Comment:
```suggestion
if logical_date is None and data_interval is not None:
raise ValueError("data_interval must be None if logical_date is None")
```
##########
airflow/jobs/scheduler_job_runner.py:
##########
@@ -1396,7 +1396,7 @@ def _create_dag_runs_asset_triggered(
dag_run = dag.create_dagrun(
run_id=dag.timetable.generate_run_id(
run_type=DagRunType.ASSET_TRIGGERED,
- logical_date=logical_date,
+ logical_date=logical_date or dag_model.next_dagrun_create_after,
Review Comment:
Is this a tmp solution as it's been blocked by https://github.com/apache/airflow/pull/46398? If so, we need to at least add a TODO here.
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1953072789
##########
airflow/models/dag.py:
##########
@@ -1782,6 +1787,9 @@ def create_dagrun(
:meta private:
"""
logical_date = timezone.coerce_datetime(logical_date)
+ # For manual runs where logical_date is None, ensure no data_interval is set.
+ if logical_date is None:
+ data_interval = None
Review Comment:
Changed it. I am raising error now.
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1951222852
##########
airflow/api_fastapi/core_api/routes/public/dag_run.py:
##########
@@ -357,25 +357,38 @@ def trigger_dag_run(
f"DAG with dag_id: '{dag_id}' has import errors and cannot be triggered",
)
- logical_date = timezone.coerce_datetime(body.logical_date)
- coerced_logical_date = timezone.coerce_datetime(logical_date)
-
+ logical_date = pendulum.instance(body.logical_date) if body.logical_date else None
+ run_after = timezone.coerce_datetime(body.run_after)
try:
dag: DAG = request.app.state.dag_bag.get_dag(dag_id)
-
if body.data_interval_start and body.data_interval_end:
data_interval = DataInterval(
start=pendulum.instance(body.data_interval_start),
end=pendulum.instance(body.data_interval_end),
)
else:
- data_interval = dag.timetable.infer_manual_data_interval(run_after=coerced_logical_date or now)
+ if body.logical_date:
+ data_interval = dag.timetable.infer_manual_data_interval(
+ run_after=timezone.coerce_datetime(body.logical_date)
+ )
+ run_after = data_interval.end
+ else:
+ data_interval = None
+
+ if body.dag_run_id:
+ run_id = body.dag_run_id
+ else:
Review Comment:
Ideally, all that validation logic for the payload should be moved to the Serializer (pydantic model). It's just adding more LOC to the route definition, and can't be re-used. (When the payload is loaded into a `TriggerDAGRunPostBody` object, this object should be valid as is without further validation/manipulation.
--
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
Re: [PR] Change default logical date to null when manual trigger from UI [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1946539529
##########
airflow/www/views.py:
##########
@@ -2224,20 +2224,27 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION):
"warning",
)
- data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
- if not run_id:
- run_id = dag.timetable.generate_run_id(
- logical_date=logical_date,
- data_interval=data_interval,
- run_type=DagRunType.MANUAL,
- )
+ run_after = timezone.utcnow()
+ if logical_date:
+ data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
+ run_after = data_interval.end
+ if not run_id:
+ run_id = dag.timetable.generate_run_id(
+ logical_date=logical_date,
+ data_interval=data_interval,
+ run_type=DagRunType.MANUAL,
+ )
+ else:
+ data_interval = None
+ if not run_id:
+ run_id = f"manual__{timezone.utcnow().isoformat()}"
Review Comment:
But this will fail tests. So I will pass logical_date as part of tests then?
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1948438527
##########
airflow/models/dagrun.py:
##########
@@ -621,10 +623,18 @@ def find_duplicate(cls, dag_id: str, run_id: str, *, session: Session = NEW_SESS
return session.scalars(select(cls).where(cls.dag_id == dag_id, cls.run_id == run_id)).one_or_none()
@staticmethod
- def generate_run_id(run_type: DagRunType, logical_date: datetime) -> str:
- """Generate Run ID based on Run Type and logical Date."""
+ def generate_run_id(
+ run_type: DagRunType, logical_date: datetime | None, run_after: datetime = timezone.utcnow()
+ ) -> str:
Review Comment:
Same, don’t change this.
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W merged PR #46512:
URL: https://github.com/apache/airflow/pull/46512
--
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
Re: [PR] Change default logical date to null when manual trigger from UI [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1946341906
##########
airflow/www/views.py:
##########
@@ -2224,20 +2224,27 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION):
"warning",
)
- data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
- if not run_id:
- run_id = dag.timetable.generate_run_id(
- logical_date=logical_date,
- data_interval=data_interval,
- run_type=DagRunType.MANUAL,
- )
+ run_after = timezone.utcnow()
+ if logical_date:
+ data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
+ run_after = data_interval.end
+ if not run_id:
+ run_id = dag.timetable.generate_run_id(
+ logical_date=logical_date,
+ data_interval=data_interval,
+ run_type=DagRunType.MANUAL,
+ )
+ else:
+ data_interval = None
+ if not run_id:
+ run_id = f"manual__{timezone.utcnow().isoformat()}"
Review Comment:
@uranusjr Should `run_after` be passed as part of the trigger method, or can it be `timezone.utcnow()`?
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1953841686
##########
airflow/models/dagrun.py:
##########
@@ -253,12 +253,14 @@ def __init__(
dag_version: DagVersion | None = None,
bundle_version: str | None = None,
):
+ # For manual runs where logical_date is None, ensure no data_interval is set.
+ if logical_date is None and data_interval is not None:
+ raise ValueError("data_interval must be None if logical_date is None")
+
if data_interval is None:
- # Legacy: Only happen for runs created prior to Airflow 2.2.
- self.data_interval_start = self.data_interval_end = None
+ self.data_interval_start, self.data_interval_end = None, None
Review Comment:
changed it back.
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1953910387
##########
airflow/api_fastapi/core_api/datamodels/dag_run.py:
##########
@@ -99,8 +99,6 @@ def check_data_intervals(cls, values):
)
return values
- ## when logical date is null, the run id should be generated from run_after + random string.
- # TODO: AIP83: we need to modify this validator after https://github.com/apache/airflow/pull/46398 is merged
Review Comment:
@vatsrahul1001 What should we do here?
--
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
Re: [PR] Change default logical date to null when manual trigger from UI [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1946350271
##########
airflow/www/views.py:
##########
@@ -2224,20 +2224,27 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION):
"warning",
)
- data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
- if not run_id:
- run_id = dag.timetable.generate_run_id(
- logical_date=logical_date,
- data_interval=data_interval,
- run_type=DagRunType.MANUAL,
- )
+ run_after = timezone.utcnow()
+ if logical_date:
+ data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
+ run_after = data_interval.end
+ if not run_id:
+ run_id = dag.timetable.generate_run_id(
+ logical_date=logical_date,
+ data_interval=data_interval,
+ run_type=DagRunType.MANUAL,
+ )
+ else:
+ data_interval = None
+ if not run_id:
+ run_id = f"manual__{timezone.utcnow().isoformat()}"
Review Comment:
Sure
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1948420241
##########
airflow/api_connexion/schemas/dag_run_schema.py:
##########
@@ -78,17 +79,19 @@ class Meta:
@pre_load
def autogenerate(self, data, **kwargs):
- """Auto generate run_id and logical_date if they are not provided."""
- logical_date = data.get("logical_date", _MISSING)
+ """Auto generate run_id, logical_date and run_after if they are not provided."""
+ run_after = data.get("run_after", _MISSING)
# Auto-generate logical_date if missing
- if logical_date is _MISSING:
- data["logical_date"] = str(timezone.utcnow())
+ if run_after is _MISSING:
+ data["run_after"] = str(timezone.utcnow())
Review Comment:
This comment is out of date.
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1953855794
##########
airflow/models/dagrun.py:
##########
@@ -253,12 +253,14 @@ def __init__(
dag_version: DagVersion | None = None,
bundle_version: str | None = None,
):
+ # For manual runs where logical_date is None, ensure no data_interval is set.
+ if logical_date is None and data_interval is not None:
+ raise ValueError("data_interval must be None if logical_date is None")
+
if data_interval is None:
- # Legacy: Only happen for runs created prior to Airflow 2.2.
- self.data_interval_start = self.data_interval_end = None
+ self.data_interval_start, self.data_interval_end = None, None
Review Comment:
I have changed it to the following now:
```
if data_interval is None:
# Legacy: Only happen for runs created prior to Airflow 2.2.
self.data_interval_start = None
self.data_interval_end = None
```
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1952026574
##########
airflow/api_connexion/schemas/dag_run_schema.py:
##########
@@ -78,17 +79,19 @@ class Meta:
@pre_load
def autogenerate(self, data, **kwargs):
- """Auto generate run_id and logical_date if they are not provided."""
- logical_date = data.get("logical_date", _MISSING)
+ """Auto generate run_id, logical_date and run_after if they are not provided."""
+ run_after = data.get("run_after", _MISSING)
# Auto-generate logical_date if missing
- if logical_date is _MISSING:
- data["logical_date"] = str(timezone.utcnow())
+ if run_after is _MISSING:
+ data["run_after"] = str(timezone.utcnow())
Review Comment:
This is changed now in https://github.com/apache/airflow/pull/46616
##########
airflow/api/common/trigger_dag.py:
##########
@@ -64,27 +64,31 @@ def _trigger_dag(
if dag is None or dag_id not in dag_bag.dags:
raise DagNotFound(f"Dag id {dag_id} not found")
-
- logical_date = logical_date or timezone.utcnow()
-
- if not timezone.is_localized(logical_date):
- raise ValueError("The logical date should be localized")
-
- if replace_microseconds:
- logical_date = logical_date.replace(microsecond=0)
-
- if dag.default_args and "start_date" in dag.default_args:
- min_dag_start_date = dag.default_args["start_date"]
- if min_dag_start_date and logical_date < min_dag_start_date:
- raise ValueError(
- f"Logical date [{logical_date.isoformat()}] should be >= start_date "
- f"[{min_dag_start_date.isoformat()}] from DAG's default_args"
- )
- coerced_logical_date = timezone.coerce_datetime(logical_date)
-
- data_interval = dag.timetable.infer_manual_data_interval(run_after=coerced_logical_date)
+ if logical_date:
+ if not timezone.is_localized(logical_date):
+ raise ValueError("The logical date should be localized")
+
+ if replace_microseconds:
+ logical_date = logical_date.replace(microsecond=0)
+
+ if dag.default_args and "start_date" in dag.default_args:
+ min_dag_start_date = dag.default_args["start_date"]
+ if min_dag_start_date and logical_date < min_dag_start_date:
+ raise ValueError(
+ f"Logical date [{logical_date.isoformat()}] should be >= start_date "
+ f"[{min_dag_start_date.isoformat()}] from DAG's default_args"
+ )
+ run_after = timezone.coerce_datetime(logical_date)
+ data_interval = dag.timetable.infer_manual_data_interval(run_after=run_after)
+ else:
+ data_interval = None
+ run_after = timezone.coerce_datetime(timezone.utcnow())
Review Comment:
This is outdated change.
##########
airflow/jobs/scheduler_job_runner.py:
##########
@@ -1396,7 +1396,7 @@ def _create_dag_runs_asset_triggered(
dag_run = dag.create_dagrun(
run_id=dag.timetable.generate_run_id(
run_type=DagRunType.ASSET_TRIGGERED,
- logical_date=logical_date,
+ logical_date=logical_date or dag_model.next_dagrun_create_after,
Review Comment:
Changed it back as it merged now
##########
airflow/www/views.py:
##########
@@ -2224,20 +2224,27 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION):
"warning",
)
- data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
- if not run_id:
- run_id = dag.timetable.generate_run_id(
- logical_date=logical_date,
- data_interval=data_interval,
- run_type=DagRunType.MANUAL,
- )
+ run_after = timezone.utcnow()
+ if logical_date:
+ data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
+ run_after = data_interval.end
+ if not run_id:
+ run_id = dag.timetable.generate_run_id(
+ logical_date=logical_date,
+ data_interval=data_interval,
+ run_type=DagRunType.MANUAL,
+ )
+ else:
+ data_interval = None
+ if not run_id:
+ run_id = f"manual__{timezone.utcnow().isoformat()}"
Review Comment:
changed it
##########
airflow/timetables/base.py:
##########
@@ -281,8 +282,17 @@ def generate_run_id(
self,
*,
run_type: DagRunType,
- logical_date: DateTime,
+ logical_date: DateTime | None,
data_interval: DataInterval | None,
+ run_after: DateTime = timezone.coerce_datetime(timezone.utcnow()),
Review Comment:
This is changed now in https://github.com/apache/airflow/pull/46616
##########
airflow/models/dag.py:
##########
@@ -1613,6 +1614,7 @@ def test(
"""
Execute one single DagRun for a given DAG and logical date.
+ :param run_after: DagRun to execute after this DagRun
Review Comment:
Outdated change. Not part of this PR anymore.
##########
airflow/api_fastapi/core_api/datamodels/dag_run.py:
##########
@@ -84,6 +84,8 @@ class TriggerDAGRunPostBody(StrictBaseModel):
dag_run_id: str | None = None
data_interval_start: AwareDatetime | None = None
data_interval_end: AwareDatetime | None = None
+ logical_date: datetime | None = None
+ run_after: datetime | None = timezone.utcnow()
Review Comment:
Removed it
##########
airflow/api_fastapi/core_api/routes/public/dag_run.py:
##########
@@ -354,18 +355,23 @@ def trigger_dag_run(
f"DAG with dag_id: '{dag_id}' has import errors and cannot be triggered",
)
- logical_date = pendulum.instance(body.logical_date)
-
+ logical_date = pendulum.instance(body.logical_date) if body.logical_date else None
+ run_after = timezone.coerce_datetime(timezone.utcnow())
Review Comment:
Changed in https://github.com/apache/airflow/pull/46616
##########
airflow/api_connexion/schemas/dag_run_schema.py:
##########
@@ -78,7 +78,7 @@ class Meta:
@pre_load
def autogenerate(self, data, **kwargs):
- """Auto generate run_id and logical_date if they are not provided."""
+ """Auto generate run_id, logical_date if they are not provided."""
Review Comment:
This is an outdated change. Not part of this PR anymore
##########
airflow/models/dag.py:
##########
@@ -1782,6 +1789,10 @@ def create_dagrun(
:meta private:
"""
logical_date = timezone.coerce_datetime(logical_date)
+ # For manual runs where logical_date is None, ensure no data_interval is set.
+ if logical_date is None:
+ if data_interval is not None:
+ raise ValueError("data_interval must be None when logical_date is None")
Review Comment:
changed it
##########
airflow/models/dagrun.py:
##########
@@ -253,12 +253,15 @@ def __init__(
dag_version: DagVersion | None = None,
bundle_version: str | None = None,
):
+ # For manual runs where logical_date is None, ensure no data_interval is set.
+ if logical_date is None:
+ if data_interval is not None:
+ raise ValueError("data_interval must be None if logical_date is None")
Review Comment:
Changed it
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1948533602
##########
airflow/timetables/base.py:
##########
@@ -281,8 +282,17 @@ def generate_run_id(
self,
*,
run_type: DagRunType,
- logical_date: DateTime,
+ logical_date: DateTime | None,
data_interval: DataInterval | None,
+ run_after: DateTime = timezone.coerce_datetime(timezone.utcnow()),
Review Comment:
changed it now. Removed run_after from here completely. I will change this after https://github.com/apache/airflow/pull/46398 is merged
##########
airflow/models/dag.py:
##########
@@ -1603,6 +1603,7 @@ def cli(self):
def test(
self,
logical_date: datetime | None = None,
+ run_after: datetime = timezone.utcnow(),
Review Comment:
changed it
##########
airflow/api_fastapi/core_api/routes/public/dag_run.py:
##########
@@ -354,18 +355,23 @@ def trigger_dag_run(
f"DAG with dag_id: '{dag_id}' has import errors and cannot be triggered",
)
- logical_date = pendulum.instance(body.logical_date)
-
+ logical_date = pendulum.instance(body.logical_date) if body.logical_date else None
+ run_after = timezone.coerce_datetime(timezone.utcnow())
Review Comment:
changed it.
##########
airflow/models/dag.py:
##########
@@ -2485,12 +2493,12 @@ def _get_or_create_dagrun(
*,
dag: DAG,
run_id: str,
- logical_date: datetime,
- data_interval: tuple[datetime, datetime],
+ logical_date: datetime | None,
+ data_interval: tuple[datetime, datetime] | None,
run_after: datetime,
conf: dict | None,
triggered_by: DagRunTriggeredByType,
- start_date: datetime,
+ start_date: datetime | None = None,
Review Comment:
changed it
##########
airflow/models/dagrun.py:
##########
@@ -621,10 +623,18 @@ def find_duplicate(cls, dag_id: str, run_id: str, *, session: Session = NEW_SESS
return session.scalars(select(cls).where(cls.dag_id == dag_id, cls.run_id == run_id)).one_or_none()
@staticmethod
- def generate_run_id(run_type: DagRunType, logical_date: datetime) -> str:
- """Generate Run ID based on Run Type and logical Date."""
+ def generate_run_id(
+ run_type: DagRunType, logical_date: datetime | None, run_after: datetime = timezone.utcnow()
+ ) -> str:
Review Comment:
changed this
##########
airflow/api_connexion/schemas/dag_run_schema.py:
##########
@@ -78,17 +79,19 @@ class Meta:
@pre_load
def autogenerate(self, data, **kwargs):
- """Auto generate run_id and logical_date if they are not provided."""
- logical_date = data.get("logical_date", _MISSING)
+ """Auto generate run_id, logical_date and run_after if they are not provided."""
+ run_after = data.get("run_after", _MISSING)
# Auto-generate logical_date if missing
- if logical_date is _MISSING:
- data["logical_date"] = str(timezone.utcnow())
+ if run_after is _MISSING:
+ data["run_after"] = str(timezone.utcnow())
Review Comment:
changed it
##########
airflow/models/dagrun.py:
##########
@@ -253,12 +253,14 @@ def __init__(
dag_version: DagVersion | None = None,
bundle_version: str | None = None,
):
+ # For manual runs where logical_date is None, ensure no data_interval is set.
+ if logical_date is None:
+ data_interval = None
Review Comment:
Changed it.
--
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
Re: [PR] Change default logical date to null when manual trigger from UI [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on PR #46512:
URL: https://github.com/apache/airflow/pull/46512#issuecomment-2639915027
We should probably not change the old UI anymore (we're very close to removing it), but we should change the New FastAPI routes 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
Re: [PR] Change default logical date to null when manual trigger from UI [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1946083662
##########
airflow/www/views.py:
##########
@@ -2224,20 +2224,27 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION):
"warning",
)
- data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
- if not run_id:
- run_id = dag.timetable.generate_run_id(
- logical_date=logical_date,
- data_interval=data_interval,
- run_type=DagRunType.MANUAL,
- )
+ run_after = timezone.utcnow()
+ if logical_date:
+ data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
+ run_after = data_interval.end
+ if not run_id:
+ run_id = dag.timetable.generate_run_id(
+ logical_date=logical_date,
+ data_interval=data_interval,
+ run_type=DagRunType.MANUAL,
+ )
+ else:
+ data_interval = None
+ if not run_id:
+ run_id = f"manual__{timezone.utcnow().isoformat()}"
Review Comment:
```suggestion
if logical_date:
data_interval = dag.timetable.infer_manual_data_interval(run_after=run_after)
run_after = data_interval.end
else:
data_interval = None
run_after = timezone.utcnow()
if not run_id:
run_id = dag.timetable.generate_run_id(
logical_date=logical_date,
data_interval=data_interval,
run_type=DagRunType.MANUAL,
)
```
Let’s not change the run_id part for now; I think @prabhusneha’s PR touches this.
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1948435913
##########
airflow/api_fastapi/core_api/routes/public/dag_run.py:
##########
@@ -354,18 +355,23 @@ def trigger_dag_run(
f"DAG with dag_id: '{dag_id}' has import errors and cannot be triggered",
)
- logical_date = pendulum.instance(body.logical_date)
-
+ logical_date = pendulum.instance(body.logical_date) if body.logical_date else None
+ run_after = timezone.coerce_datetime(timezone.utcnow())
Review Comment:
This should use run_after from the post body.
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1953917418
##########
airflow/cli/commands/remote_commands/task_command.py:
##########
@@ -129,28 +127,32 @@ def _get_dag_run(
f"of {logical_date_or_run_id!r} not found"
)
- dag_run_logical_date = pendulum.instance(logical_date or timezone.utcnow())
+ dag_run_logical_date = timezone.coerce_datetime(logical_date) if logical_date else None
Review Comment:
TIL: it handles None 🤔
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1953916335
##########
airflow/models/dagrun.py:
##########
@@ -253,12 +253,14 @@ def __init__(
dag_version: DagVersion | None = None,
bundle_version: str | None = None,
):
+ # For manual runs where logical_date is None, ensure no data_interval is set.
+ if logical_date is None and data_interval is not None:
+ raise ValueError("data_interval must be None if logical_date is None")
+
if data_interval is None:
- # Legacy: Only happen for runs created prior to Airflow 2.2.
- self.data_interval_start = self.data_interval_end = None
+ self.data_interval_start, self.data_interval_end = None, None
Review Comment:
@Lee-W I have changed it back to
```
self.data_interval_start, self.data_interval_end = None, None
``` as discussed on slack
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1953918509
##########
airflow/cli/commands/remote_commands/task_command.py:
##########
@@ -129,28 +127,32 @@ def _get_dag_run(
f"of {logical_date_or_run_id!r} not found"
)
- dag_run_logical_date = pendulum.instance(logical_date or timezone.utcnow())
+ dag_run_logical_date = timezone.coerce_datetime(logical_date) if logical_date else None
Review Comment:
Right. I checked it now as well.
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1948437831
##########
airflow/models/dag.py:
##########
@@ -1782,6 +1787,9 @@ def create_dagrun(
:meta private:
"""
logical_date = timezone.coerce_datetime(logical_date)
+ # For manual runs where logical_date is None, ensure no data_interval is set.
+ if logical_date is None:
+ data_interval = None
Review Comment:
This should be ensured at the _call site_. This function can raise an error if data interval is provided when logical date is None, although I think even that’s not necessary. It does not make sense, but we can allow it.
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1948436569
##########
airflow/models/dag.py:
##########
@@ -1603,6 +1603,7 @@ def cli(self):
def test(
self,
logical_date: datetime | None = None,
+ run_after: datetime = timezone.utcnow(),
Review Comment:
Can not do this.
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1948422201
##########
airflow/api_fastapi/core_api/datamodels/dag_run.py:
##########
@@ -84,6 +84,8 @@ class TriggerDAGRunPostBody(StrictBaseModel):
dag_run_id: str | None = None
data_interval_start: AwareDatetime | None = None
data_interval_end: AwareDatetime | None = None
+ logical_date: datetime | None = None
+ run_after: datetime | None = timezone.utcnow()
Review Comment:
We can’t do this because the value is evaluated at _import time_ and everything ends up having the same run_after value. `utcnow` needs to be lazily called.
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1952008072
##########
airflow/jobs/scheduler_job_runner.py:
##########
@@ -1396,7 +1396,7 @@ def _create_dag_runs_asset_triggered(
dag_run = dag.create_dagrun(
run_id=dag.timetable.generate_run_id(
run_type=DagRunType.ASSET_TRIGGERED,
- logical_date=logical_date,
+ logical_date=logical_date or dag_model.next_dagrun_create_after,
Review Comment:
This is being handled in your PR i think: https://github.com/apache/airflow/pull/46460
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on PR #46512:
URL: https://github.com/apache/airflow/pull/46512#issuecomment-2655447159
opps, looks like there's conflict
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1953914647
##########
airflow/api_fastapi/core_api/datamodels/dag_run.py:
##########
@@ -99,8 +99,6 @@ def check_data_intervals(cls, values):
)
return values
- ## when logical date is null, the run id should be generated from run_after + random string.
- # TODO: AIP83: we need to modify this validator after https://github.com/apache/airflow/pull/46398 is merged
Review Comment:
I have changed it already in https://github.com/apache/airflow/pull/46616
You can see the diff here: https://github.com/apache/airflow/pull/46616/files#diff-2046c053c5d62417377a58df31e9432f143bcbed09ad1656382b3b5e09b9263bR104-R108
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1948419526
##########
airflow/timetables/base.py:
##########
@@ -281,8 +282,17 @@ def generate_run_id(
self,
*,
run_type: DagRunType,
- logical_date: DateTime,
+ logical_date: DateTime | None,
data_interval: DataInterval | None,
+ run_after: DateTime = timezone.coerce_datetime(timezone.utcnow()),
Review Comment:
Let’s not change this signature now. For cases with None logical_date, let’s just do
```python
logical_date=logical_date or run_after
```
to keep things work.
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1948438111
##########
airflow/models/dag.py:
##########
@@ -2485,12 +2493,12 @@ def _get_or_create_dagrun(
*,
dag: DAG,
run_id: str,
- logical_date: datetime,
- data_interval: tuple[datetime, datetime],
+ logical_date: datetime | None,
+ data_interval: tuple[datetime, datetime] | None,
run_after: datetime,
conf: dict | None,
triggered_by: DagRunTriggeredByType,
- start_date: datetime,
+ start_date: datetime | None = None,
Review Comment:
Why does this need to change?
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1948438340
##########
airflow/models/dagrun.py:
##########
@@ -253,12 +253,14 @@ def __init__(
dag_version: DagVersion | None = None,
bundle_version: str | None = None,
):
+ # For manual runs where logical_date is None, ensure no data_interval is set.
+ if logical_date is None:
+ data_interval = None
Review Comment:
Same, we should not do this.
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1952008072
##########
airflow/jobs/scheduler_job_runner.py:
##########
@@ -1396,7 +1396,7 @@ def _create_dag_runs_asset_triggered(
dag_run = dag.create_dagrun(
run_id=dag.timetable.generate_run_id(
run_type=DagRunType.ASSET_TRIGGERED,
- logical_date=logical_date,
+ logical_date=logical_date or dag_model.next_dagrun_create_after,
Review Comment:
This is being handled in your PR i think: https://github.com/apache/airflow/pull/46460
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1953841686
##########
airflow/models/dagrun.py:
##########
@@ -253,12 +253,14 @@ def __init__(
dag_version: DagVersion | None = None,
bundle_version: str | None = None,
):
+ # For manual runs where logical_date is None, ensure no data_interval is set.
+ if logical_date is None and data_interval is not None:
+ raise ValueError("data_interval must be None if logical_date is None")
+
if data_interval is None:
- # Legacy: Only happen for runs created prior to Airflow 2.2.
- self.data_interval_start = self.data_interval_end = None
+ self.data_interval_start, self.data_interval_end = None, None
Review Comment:
When I try to change it back ruff throws the following error:
```
airflow/models/dagrun.py:262: error: "None" object is not iterable [misc]
self.data_interval_start, self.data_interval_end = None
^~~~
```
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1953911099
##########
airflow/cli/commands/remote_commands/task_command.py:
##########
@@ -129,28 +127,32 @@ def _get_dag_run(
f"of {logical_date_or_run_id!r} not found"
)
- dag_run_logical_date = pendulum.instance(logical_date or timezone.utcnow())
+ dag_run_logical_date = timezone.coerce_datetime(logical_date) if logical_date else None
Review Comment:
```suggestion
dag_run_logical_date = timezone.coerce_datetime(logical_date)
```
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1953070837
##########
airflow/api_fastapi/core_api/routes/public/dag_run.py:
##########
@@ -357,25 +357,38 @@ def trigger_dag_run(
f"DAG with dag_id: '{dag_id}' has import errors and cannot be triggered",
)
- logical_date = timezone.coerce_datetime(body.logical_date)
- coerced_logical_date = timezone.coerce_datetime(logical_date)
-
+ logical_date = pendulum.instance(body.logical_date) if body.logical_date else None
+ run_after = timezone.coerce_datetime(body.run_after)
try:
dag: DAG = request.app.state.dag_bag.get_dag(dag_id)
-
if body.data_interval_start and body.data_interval_end:
data_interval = DataInterval(
start=pendulum.instance(body.data_interval_start),
end=pendulum.instance(body.data_interval_end),
)
else:
- data_interval = dag.timetable.infer_manual_data_interval(run_after=coerced_logical_date or now)
+ if body.logical_date:
+ data_interval = dag.timetable.infer_manual_data_interval(
+ run_after=timezone.coerce_datetime(body.logical_date)
+ )
+ run_after = data_interval.end
+ else:
+ data_interval = None
+
+ if body.dag_run_id:
+ run_id = body.dag_run_id
+ else:
Review Comment:
I did the API change as part of [46616](https://github.com/apache/airflow/pull/46616).
But I can take care of this in issue: [#46650](https://github.com/apache/airflow/issues/46650)
--
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
Re: [PR] Change default logical date to null when manual trigger from UI [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1946539529
##########
airflow/www/views.py:
##########
@@ -2224,20 +2224,27 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION):
"warning",
)
- data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
- if not run_id:
- run_id = dag.timetable.generate_run_id(
- logical_date=logical_date,
- data_interval=data_interval,
- run_type=DagRunType.MANUAL,
- )
+ run_after = timezone.utcnow()
+ if logical_date:
+ data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
+ run_after = data_interval.end
+ if not run_id:
+ run_id = dag.timetable.generate_run_id(
+ logical_date=logical_date,
+ data_interval=data_interval,
+ run_type=DagRunType.MANUAL,
+ )
+ else:
+ data_interval = None
+ if not run_id:
+ run_id = f"manual__{timezone.utcnow().isoformat()}"
Review Comment:
But this will fail tests. So I will pass logical_date as part of tests then
--
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
Re: [PR] Change default logical date to null when manual trigger from UI [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1946341906
##########
airflow/www/views.py:
##########
@@ -2224,20 +2224,27 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION):
"warning",
)
- data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
- if not run_id:
- run_id = dag.timetable.generate_run_id(
- logical_date=logical_date,
- data_interval=data_interval,
- run_type=DagRunType.MANUAL,
- )
+ run_after = timezone.utcnow()
+ if logical_date:
+ data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
+ run_after = data_interval.end
+ if not run_id:
+ run_id = dag.timetable.generate_run_id(
+ logical_date=logical_date,
+ data_interval=data_interval,
+ run_type=DagRunType.MANUAL,
+ )
+ else:
+ data_interval = None
+ if not run_id:
+ run_id = f"manual__{timezone.utcnow().isoformat()}"
Review Comment:
@uranusjr Should `run_after` be passed as part of the trigger method, or can it be `timezone.utcnow()`?
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1946350271
##########
airflow/www/views.py:
##########
@@ -2224,20 +2224,27 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION):
"warning",
)
- data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
- if not run_id:
- run_id = dag.timetable.generate_run_id(
- logical_date=logical_date,
- data_interval=data_interval,
- run_type=DagRunType.MANUAL,
- )
+ run_after = timezone.utcnow()
+ if logical_date:
+ data_interval = dag.timetable.infer_manual_data_interval(run_after=logical_date)
+ run_after = data_interval.end
+ if not run_id:
+ run_id = dag.timetable.generate_run_id(
+ logical_date=logical_date,
+ data_interval=data_interval,
+ run_type=DagRunType.MANUAL,
+ )
+ else:
+ data_interval = None
+ if not run_id:
+ run_id = f"manual__{timezone.utcnow().isoformat()}"
Review Comment:
Sure. But this fails mypy and tests
--
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
Re: [PR] Change default logical date to null when manual trigger from UI [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #46512:
URL: https://github.com/apache/airflow/pull/46512#issuecomment-2642157092
By “soon” it’s after 3.0 right? I feel we should still keep the old view work the same as the new to not confuse users too much.
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1952034189
##########
airflow/models/dag.py:
##########
@@ -1602,6 +1602,7 @@ def cli(self):
@provide_session
def test(
self,
+ run_after: datetime | None = None,
Review Comment:
Yes it is right. For tests, when run_after is None it is being set to utcnow() here: https://github.com/apache/airflow/blob/main/airflow/models/dag.py#L1667
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on PR #46512:
URL: https://github.com/apache/airflow/pull/46512#issuecomment-2655633228
> opps, looks like there's conflict
@Lee-W fixed it
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1953991120
##########
airflow/models/dagrun.py:
##########
@@ -253,12 +253,14 @@ def __init__(
dag_version: DagVersion | None = None,
bundle_version: str | None = None,
):
+ # For manual runs where logical_date is None, ensure no data_interval is set.
+ if logical_date is None and data_interval is not None:
+ raise ValueError("data_interval must be None if logical_date is None")
+
if data_interval is None:
- # Legacy: Only happen for runs created prior to Airflow 2.2.
- self.data_interval_start = self.data_interval_end = None
+ self.data_interval_start, self.data_interval_end = None, None
Review Comment:
changed it back. It was `,` instead of `=`
--
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
Re: [PR] Change manual run data interval behavior based on logical_date [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on code in PR #46512:
URL: https://github.com/apache/airflow/pull/46512#discussion_r1953781252
##########
airflow/models/dagrun.py:
##########
@@ -253,12 +253,14 @@ def __init__(
dag_version: DagVersion | None = None,
bundle_version: str | None = None,
):
+ # For manual runs where logical_date is None, ensure no data_interval is set.
+ if logical_date is None and data_interval is not None:
+ raise ValueError("data_interval must be None if logical_date is None")
+
if data_interval is None:
- # Legacy: Only happen for runs created prior to Airflow 2.2.
- self.data_interval_start = self.data_interval_end = None
+ self.data_interval_start, self.data_interval_end = None, None
Review Comment:
We probably don't really need to change this line. Both are readable for me. not sure how others think 🤔
--
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