You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "shreyaskj-0710 (via GitHub)" <gi...@apache.org> on 2025/08/12 05:03:15 UTC
[PR] Issue fix 53741 [airflow]
shreyaskj-0710 opened a new pull request, #54382:
URL: https://github.com/apache/airflow/pull/54382
Fixes #53741
**Description**
These changes are made to enable new query param 'is_scheduled' to /dags api.
**Reason for the approach**
To determine if a DAG is scheduled, several data sources can be considered:
- serialized_dag table — _type attribute under the data column.
- dag_run table — absence of records may indicate no schedule.
- dag table — timetable_description column.
After evaluating these options, the dag.timetable_description column seems to be more time efficient . This PR
implements the logic using that column.
**testing**
Verified the API using Postman by creating both scheduled and unscheduled DAGs, then confirming that they were
correctly filtered by setting the is_scheduled parameter to true or false.
--
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] Issue fix 53741 [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #54382:
URL: https://github.com/apache/airflow/pull/54382#discussion_r2273909813
##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -540,6 +540,22 @@ def is_active(self) -> bool:
self.value.lower_bound is not None or self.value.upper_bound is not None
)
+class _IsDagScheduledFilter(BaseParam[bool]):
+ """Filter on timetable_description."""
+
+ def to_orm(self, select: Select) -> Select:
+ if not self.value is None and self.skip_none:
+ if not self.value:
+ return select.where(DagModel.timetable_description.ilike("%Never%"))
+ else:
+ return select.where(DagModel.timetable_description.not_like("%Never%"))
Review Comment:
I have a little concern about using `timetable_description` to filter whether the dag is scheduled or not, but it seems which is the only way to filter at db level.
This _should_ be no problem for most case, as there will only be `Never` substring in `NullTimetable`.
https://github.com/apache/airflow/blob/81538ef9217093c51aeeb29747fe4796cf89c550/airflow-core/src/airflow/timetables/simple.py#L60-L68
##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -540,6 +540,22 @@ def is_active(self) -> bool:
self.value.lower_bound is not None or self.value.upper_bound is not None
)
+class _IsDagScheduledFilter(BaseParam[bool]):
+ """Filter on timetable_description."""
+
+ def to_orm(self, select: Select) -> Select:
+ if not self.value is None and self.skip_none:
+ if not self.value:
+ return select.where(DagModel.timetable_description.ilike("%Never%"))
+ else:
+ return select.where(DagModel.timetable_description.not_like("%Never%"))
Review Comment:
A small speedup for query
```suggestion
return select.where(DagModel.timetable_description.ilike("Never%"))
else:
return select.where(DagModel.timetable_description.not_like("Never%"))
```
--
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] Issue fix 53741 [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #54382:
URL: https://github.com/apache/airflow/pull/54382#issuecomment-3182886030
Could you please also update the unit test to test these conditions? :)
--
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] Add query param is_scheduled to /dags api [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #54382:
URL: https://github.com/apache/airflow/pull/54382#issuecomment-3368624213
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
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] Issue fix 53741 [airflow]
Posted by "shreyaskj-0710 (via GitHub)" <gi...@apache.org>.
shreyaskj-0710 commented on code in PR #54382:
URL: https://github.com/apache/airflow/pull/54382#discussion_r2269367713
##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -540,6 +540,22 @@ def is_active(self) -> bool:
self.value.lower_bound is not None or self.value.upper_bound is not None
)
+class _IsDagScheduledFilter(BaseParam[bool]):
+ """Filter on timetable_description."""
+
+ def to_orm(self, select: Select) -> Select:
+ if not self.value is None and self.skip_none:
+ if not self.value:
Review Comment:
Hi @bugraoz93 ,
This condition is required because the is_scheduled query parameter is optional.
The is_scheduled parameter can take one of three values:
- True → retrieve scheduled DAGs
- False → retrieve unscheduled DAGs
- None → when is_scheduled is not provided in the query parameters.
Therefore, the check 'if self.value is not None' will be True when is_scheduled is explicitly set to True or False, and False when the parameter is omitted from the API request.
if not self.value will be checked when is_scheduled is explicitly set to True or False.
Code snippet :
if self.value is not None (or if not self.value is None)
-> Filters out the case where is_scheduled is not provided in the query.
if not self.value (inside)
-> Specifically handles the case where is_scheduled=False (unscheduled DAGs).
else (inside)
-> Handles the case where is_scheduled=True (scheduled DAGs).
--
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] Issue fix 53741 [airflow]
Posted by "shreyaskj-0710 (via GitHub)" <gi...@apache.org>.
shreyaskj-0710 commented on code in PR #54382:
URL: https://github.com/apache/airflow/pull/54382#discussion_r2276006425
##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -540,6 +540,22 @@ def is_active(self) -> bool:
self.value.lower_bound is not None or self.value.upper_bound is not None
)
+class _IsDagScheduledFilter(BaseParam[bool]):
+ """Filter on timetable_description."""
+
+ def to_orm(self, select: Select) -> Select:
+ if not self.value is None and self.skip_none:
+ if not self.value:
+ return select.where(DagModel.timetable_description.ilike("%Never%"))
+ else:
+ return select.where(DagModel.timetable_description.not_like("%Never%"))
Review Comment:
Hi @jason810496 ,
have implemented suggested changes,
please check.
--
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] Add query param is_scheduled to /dags api [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun closed pull request #54382: Add query param is_scheduled to /dags api
URL: https://github.com/apache/airflow/pull/54382
--
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] Issue fix 53741 [airflow]
Posted by "shreyaskj-0710 (via GitHub)" <gi...@apache.org>.
shreyaskj-0710 commented on PR #54382:
URL: https://github.com/apache/airflow/pull/54382#issuecomment-3187629806
> Could you please also update the unit test to test these conditions? :)
Have written test cases as well, please check
--
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] Issue fix 53741 [airflow]
Posted by "shreyaskj-0710 (via GitHub)" <gi...@apache.org>.
shreyaskj-0710 commented on PR #54382:
URL: https://github.com/apache/airflow/pull/54382#issuecomment-3183134136
> Could you please also update the unit test to test these conditions? :)
I’m not very familiar with pytest/unittest, but I’ll give it a try.
--
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] Add query param is_scheduled to /dags api [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #54382:
URL: https://github.com/apache/airflow/pull/54382#discussion_r2288056266
##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -540,6 +540,22 @@ def is_active(self) -> bool:
self.value.lower_bound is not None or self.value.upper_bound is not None
)
+class _IsDagScheduledFilter(BaseParam[bool]):
+ """Filter on timetable_description."""
+
+ def to_orm(self, select: Select) -> Select:
+ if not self.value is None and self.skip_none:
+ if not self.value:
Review Comment:
I had the exact first idea. Maybe we should refactor it so it's less confusing. We don't want to bother handling the `None` expliciit filtering for now. Pseudo code not tested:
```python
if self.skip_none is False:
raise ValueError(f"Cannot set 'skip_none' to False on a {type(self)}")
if self.value is None:
return select
if self.value:
return select.where(DagModel.timetable_description.ilike("Never%"))|
else:
return select.where(DagModel.timetable_description.not_like("Never%"))
```
--
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] Add query param is_scheduled to /dags api [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #54382:
URL: https://github.com/apache/airflow/pull/54382#issuecomment-3587562384
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
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] Issue fix 53741 [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #54382:
URL: https://github.com/apache/airflow/pull/54382#discussion_r2269254651
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##########
@@ -89,6 +90,7 @@ def get_dags(
last_dag_run_state: QueryLastDagRunStateFilter,
bundle_name: QueryBundleNameFilter,
bundle_version: QueryBundleVersionFilter,
+ is_scheduled:QueryIsDagScheduledFilter,
Review Comment:
```suggestion
is_scheduled: QueryIsDagScheduledFilter,
```
##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -540,6 +540,22 @@ def is_active(self) -> bool:
self.value.lower_bound is not None or self.value.upper_bound is not None
)
+class _IsDagScheduledFilter(BaseParam[bool]):
+ """Filter on timetable_description."""
+
+ def to_orm(self, select: Select) -> Select:
+ if not self.value is None and self.skip_none:
+ if not self.value:
Review Comment:
You have already added `if not self.value is None` above which makes this `if not self.value` always true. We may either need two if separately if they are separate cases or merge them into one could be make sense too
--
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] Issue fix 53741 [airflow]
Posted by "shreyaskj-0710 (via GitHub)" <gi...@apache.org>.
shreyaskj-0710 commented on code in PR #54382:
URL: https://github.com/apache/airflow/pull/54382#discussion_r2275568683
##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -540,6 +540,22 @@ def is_active(self) -> bool:
self.value.lower_bound is not None or self.value.upper_bound is not None
)
+class _IsDagScheduledFilter(BaseParam[bool]):
+ """Filter on timetable_description."""
+
+ def to_orm(self, select: Select) -> Select:
+ if not self.value is None and self.skip_none:
+ if not self.value:
+ return select.where(DagModel.timetable_description.ilike("%Never%"))
+ else:
+ return select.where(DagModel.timetable_description.not_like("%Never%"))
Review Comment:
Yes, I have tested this case in Postman, and it’s working fine.
--
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] Add query param is_scheduled to /dags api [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #54382:
URL: https://github.com/apache/airflow/pull/54382#discussion_r2426843198
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##########
@@ -146,6 +148,7 @@ def get_dags(
readable_dags_filter,
bundle_name,
bundle_version,
+ is_scheduled,
Review Comment:
Similarly can you also update the private `ui/dags` endpoint. `get_dags` because the UI uses its own custom endpoint and the filter need to be there too.
##########
airflow-core/tests/unit/api_fastapi/common/test_parameters.py:
##########
@@ -38,3 +40,67 @@ def test_sort_param_max_number_of_filers(self):
),
):
param.to_orm(None)
+
+
+class TestIsDagScheduledFilter:
+ @pytest.fixture
+ def mock_dagmodel(self):
+ """
+ Patch DagModel in the parameters module's namespace so the filter uses our mock.
+ """
+ with patch("airflow.api_fastapi.common.parameters.DagModel", autospec=True) as mock_dm:
+ timetable_mock = MagicMock()
+ mock_dm.timetable_description = timetable_mock
+ yield mock_dm
+
+ @pytest.fixture
+ def mock_select(self):
+ """
+ Create a mock SQLAlchemy Select with .where().
+ """
+ select = MagicMock(spec=Select)
+ select.where.return_value = "filtered"
+ return select
+
+ def test_to_orm_value_none_returns_original(self, mock_select):
+ f = _IsDagScheduledFilter()
+ f.value = None
+ f.skip_none = True
+
+ result = f.to_orm(mock_select)
+
+ assert result == mock_select
+ mock_select.where.assert_not_called()
+
+ def test_to_orm_false_applies_never_filter(self, mock_select, mock_dagmodel):
+ f = _IsDagScheduledFilter()
+ f.value = False
+ f.skip_none = True
+
+ mock_condition = MagicMock(name="ilike_condition")
+ mock_dagmodel.timetable_description.ilike.return_value = mock_condition
+
+ result = f.to_orm(mock_select)
+
+ assert result == "filtered"
+ mock_dagmodel.timetable_description.ilike.assert_called_once_with("Never%")
+ mock_select.where.assert_called_once_with(mock_condition)
+
+ def test_to_orm_true_applies_not_never_filter(self, mock_select, mock_dagmodel):
+ f = _IsDagScheduledFilter()
+ f.value = True
+ f.skip_none = True
+
+ mock_condition = MagicMock(name="not_like_condition")
+ mock_dagmodel.timetable_description.not_like.return_value = mock_condition
+
+ result = f.to_orm(mock_select)
+
+ assert result == "filtered"
+ mock_dagmodel.timetable_description.not_like.assert_called_once_with("Never%")
+ mock_select.where.assert_called_once_with(mock_condition)
+
+ def test_depends_sets_value(self):
+ f = _IsDagScheduledFilter.depends(True)
+ assert isinstance(f, _IsDagScheduledFilter)
+ assert f.value is True
Review Comment:
Instead of mocking everything, can you add remove those tests, and add one in `public/test_dags.py` `test_get_dags`.
--
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] Issue fix 53741 [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #54382:
URL: https://github.com/apache/airflow/pull/54382#discussion_r2272630879
##########
airflow-core/src/airflow/api_fastapi/common/parameters.py:
##########
@@ -540,6 +540,22 @@ def is_active(self) -> bool:
self.value.lower_bound is not None or self.value.upper_bound is not None
)
+class _IsDagScheduledFilter(BaseParam[bool]):
+ """Filter on timetable_description."""
+
+ def to_orm(self, select: Select) -> Select:
+ if not self.value is None and self.skip_none:
+ if not self.value:
Review Comment:
Thanks for the details! Indeed, I overlooked the condition, these are different conditions
--
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] Issue fix 53741 [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #54382:
URL: https://github.com/apache/airflow/pull/54382#discussion_r2279320533
##########
airflow-core/tests/unit/api_fastapi/common/test_parameters.py:
##########
@@ -38,3 +40,71 @@ def test_sort_param_max_number_of_filers(self):
),
):
param.to_orm(None)
+
+
Review Comment:
How about refactor the tests as `TestIsDagScheduledFilter` class based style and leverage `parameterize` to avoid duplication.
--
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] Add query param is_scheduled to /dags api [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on PR #54382:
URL: https://github.com/apache/airflow/pull/54382#issuecomment-3600844177
> @shreyaskj-0710 Any chance you can address the comments so we can merge the PR.
>
> That would benefit to many users waiting for this improvement :)
Hi @pierrejeambrun, this PR seems stale for a while, and current dependencies for supporting this feature will be:
1. #58669
2. #58852 , as TP pointed out that 1. is more crucial than 2. for now in the [comment](https://github.com/apache/airflow/pull/58852#pullrequestreview-3523374533).
3. #58848 , which will take over the current PR
--
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] Issue fix 53741 [airflow]
Posted by "shreyaskj-0710 (via GitHub)" <gi...@apache.org>.
shreyaskj-0710 commented on code in PR #54382:
URL: https://github.com/apache/airflow/pull/54382#discussion_r2269368285
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dags.py:
##########
@@ -89,6 +90,7 @@ def get_dags(
last_dag_run_state: QueryLastDagRunStateFilter,
bundle_name: QueryBundleNameFilter,
bundle_version: QueryBundleVersionFilter,
+ is_scheduled:QueryIsDagScheduledFilter,
Review Comment:
sure will 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] Issue fix 53741 [airflow]
Posted by "shreyaskj-0710 (via GitHub)" <gi...@apache.org>.
shreyaskj-0710 commented on code in PR #54382:
URL: https://github.com/apache/airflow/pull/54382#discussion_r2284096657
##########
airflow-core/tests/unit/api_fastapi/common/test_parameters.py:
##########
@@ -38,3 +40,71 @@ def test_sort_param_max_number_of_filers(self):
),
):
param.to_orm(None)
+
+
Review Comment:
Hi @jason810496 ,
have implemented suggested changes,
please check.
--
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] Add query param is_scheduled to /dags api [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #54382:
URL: https://github.com/apache/airflow/pull/54382#issuecomment-3660459775
> https://github.com/apache/airflow/pull/58848 , which will take over the current PR
Closing then, feel free to reopen if needed.
--
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