Fix: PATCH /dags pagination bug and document wildcard dag_id_pattern#63665
Fix: PATCH /dags pagination bug and document wildcard dag_id_pattern#63665justinpakzad wants to merge 1 commit intoapache:mainfrom
Conversation
justinpakzad
requested review from
bugraoz93,
choo121600,
ephraimbuddy,
jason810496,
pierrejeambrun,
rawwar and
shubhamraj-git
as code owners
justinpakzad
force-pushed
the
fix/dags-patch-pause-with-tags
branch
from
c032f2c to
3b2f945
Compare
potiuk
added
the
ready for maintainer review
There was a problem hiding this comment.
This is not needed. I believe you can achieve this by passing ~ or % as a wildcard dag_id_search param.
This can be detailed in the endpoint documentation (docstring that ends up in the openAPI spec) and will avoid accident or adding more code.
Also there is a pagination bug. Only the first matching 25 dags will be unpaused, we need to iterate though all pages.
Do you mind reconverting this PR to fixing the pagination issue so we actually update all the dags that match the query, and also add some docstring to explain the wildcard pattern ?
| [], | ||
| ), | ||
| ( | ||
| {"owners": ["test_owner"], "exclude_stale": False}, |
There was a problem hiding this comment.
Can you add a test case for what you are trying to solve. (match any tags, multiple tags, and multiple dags patched)?
| [ | ||
| dag_id_pattern.value is not None, | ||
| tags.value is not None and tags.value.tags, | ||
| owners.value, | ||
| ] |
There was a problem hiding this comment.
It's hard to justify why 'paused' or exclude_stale are excluded from this check. (Only some filters contribute to the check which can seem confusing, can't unpause all paused dags)
There was a problem hiding this comment.
I think I misunderstood and thought it made sense to exclude those as a safeguard so a user doesn't accidentally unpause all paused dags, but yea it's confusing. Will revert these changes since all that needs fixing is the pagination + documenting the wildcard search parameter (as you mentioned).
| dags_select, total_entries = paginated_select( | ||
| statement=select(DagModel), | ||
| filters=[ | ||
| exclude_stale, | ||
| paused, | ||
| dag_id_pattern, | ||
| tags, | ||
| owners, | ||
| editable_dags_filter, | ||
| ], | ||
| order_by=None, | ||
| offset=offset, | ||
| limit=limit, | ||
| session=session, | ||
| ) |
There was a problem hiding this comment.
Here we need to iterate through the pages to get all of them.
There was a problem hiding this comment.
Will make that change. Thanks for pointing that out.
justinpakzad
force-pushed
the
fix/dags-patch-pause-with-tags
branch
from
3b2f945 to
34995d8
Compare
justinpakzad
requested review from
bbovenzi,
guan404ming,
ryanahamilton and
vatsrahul1001
as code owners
justinpakzad
changed the title
3 participants
Currently the PATCH /dags endpoint only updates the first page of matching DAGs (default limit of 50), meaning if the filters match more than one page, only the first page gets paused/unpaused. This fixes that by iterating through all pages to collect all matching dag ids before applying the update.
Also added to the docstring to clarify that
dag_id_patternis required to match any dags, and that~or%can be passed as wildcards to match all.closes: #50555
Was generative AI tooling used to co-author this PR?
Claude Sonnet 4.5 (mainly for generating test dags)
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.