pierrejeambrun commented on code in PR #71654:
URL: https://github.com/apache/airflow/pull/71654#discussion_r3830046265
##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -1239,8 +1254,16 @@ def depends_datetime(
upper_bound_lte=upper_bound_lte,
upper_bound_lt=upper_bound_lt,
)
- if filter_name in ("start_date", "end_date"):
- return NullableDatetimeRangeFilter(range_val, attr)
+ attr_name = attribute_name or filter_name
+ if filter_name in ("start_date", "end_date") or (model is DagRun and
attr_name == "start_date"):
+ null_lower_bound_clause: ColumnElement[bool] | None = None
Review Comment:
We probably shouldn't have an `model is DagRun and attr_name ==
"start_date")`. Other models have the same problem `TaskInstance` and `Job`.
This will just grow the 'if' when we actually fix those as follow up.
##########
airflow-core/tests/unit/api_fastapi/common/test_parameters.py:
##########
@@ -504,14 +504,11 @@ def test_end_date_returns_nullable_filter(self):
rf = _make_datetime_filter("end_date")
assert isinstance(rf, NullableDatetimeRangeFilter)
- def test_aliased_filter_name_returns_plain_filter(self):
- """dag_run_start_date uses attribute_name='start_date' via outer join;
NULL means 'no run',
- not 'currently running', so it must return a plain RangeFilter to
avoid inflating counts."""
+ def test_aliased_start_date_returns_nullable_filter(self):
rf = _make_datetime_filter("dag_run_start_date", model=DagRun,
attribute_name="start_date")
- assert type(rf) is RangeFilter
+ assert isinstance(rf, NullableDatetimeRangeFilter)
Review Comment:
This was an explicit design decision and this is reverting it. Is that
intentional?
Basically before this, a dag with 'queued' latest run (not started) wouldn't
return. Now a dag with latest dag run in queued state will return for
`/dags?dag_run_start_date_gte=2026-08-21T00:00:00Z`
WDYT is that a preferable behavior?
##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -1239,8 +1254,16 @@ def depends_datetime(
upper_bound_lte=upper_bound_lte,
upper_bound_lt=upper_bound_lt,
)
- if filter_name in ("start_date", "end_date"):
- return NullableDatetimeRangeFilter(range_val, attr)
+ attr_name = attribute_name or filter_name
+ if filter_name in ("start_date", "end_date") or (model is DagRun and
attr_name == "start_date"):
+ null_lower_bound_clause: ColumnElement[bool] | None = None
Review Comment:
Maybe move that parameter to the kwarg params of the factory intead.
```python
# routes/public/dag_run.py
start_date_range: Annotated[RangeFilter,
Depends(datetime_range_filter_factory(
"start_date", DagRun,
null_lower_bound_clause=DagRun.end_date.is_(None),
))]
# routes/public/dags.py — where the LEFT JOIN makes the id guard
load-bearing
dag_run_start_date_range: Annotated[RangeFilter,
Depends(datetime_range_filter_factory(
"dag_run_start_date", DagRun, "start_date",
null_lower_bound_clause=and_(DagRun.id.is_not(None),
DagRun.end_date.is_(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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
