Skip to content

Fix DAG version inflation caused by unstable DagParam repr#65705

Open
wjddn279 wants to merge 2 commits into
apache:mainfrom
wjddn279:fix-dag-inflation-in-dag-decorator
Open

Fix DAG version inflation caused by unstable DagParam repr#65705
wjddn279 wants to merge 2 commits into
apache:mainfrom
wjddn279:fix-dag-inflation-in-dag-decorator

Conversation

@wjddn279
Copy link
Copy Markdown
Contributor

Closes: #65674

In serailized_dag, op_args of task_id transform.to_exp is changed in every parsing, because DagParam has no stringfy function

"op_args": "(MappedArgument(_input=DictOfListsExpandInput(value={'number': XComArg(<Task(_PythonDecoratedOperator): get_data>)}), _key='number'), <airflow.sdk.definitions.param.DagParam object at 0xf96d2cd44a60>)",

The template_field ends up as a tuple, but since it contains an object type, is_jsonable returns False. Because tuple has no serialize path, execution falls into this logic, and the plain string conversion embeds the raw object representation (including its id), which is highly likely to cause Dag inflation.

I means the logic needs to be modified to handle the case not is_jsonable but list, tuple, dict type like this #63871


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {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.

Comment thread task-sdk/src/airflow/sdk/definitions/param.py Outdated
Comment thread task-sdk/src/airflow/sdk/definitions/param.py Outdated
Comment thread task-sdk/src/airflow/sdk/definitions/param.py Outdated
Comment thread airflow-core/tests/unit/serialization/test_dag_serialization.py
@wjddn279 wjddn279 force-pushed the fix-dag-inflation-in-dag-decorator branch from de5a57f to a5906e3 Compare April 24, 2026 07:28
@wjddn279 wjddn279 requested a review from bolkedebruin as a code owner April 24, 2026 07:28
@wjddn279
Copy link
Copy Markdown
Contributor Author

wjddn279 commented Apr 24, 2026

@kaxil @uranusjr

I've changed it so that serialize_template_field handles the issue. I also considered @attr.define, but decided against it since it would require modifying the existing __init__

@potiuk potiuk added the ready for maintainer review Set after triaging when all criteria pass. label Apr 27, 2026
Copy link
Copy Markdown
Contributor

@ephraimbuddy ephraimbuddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. This is fixing similar issue as #63871.
Neither alone covers the union (e.g. dict-of-list-of-callable, list-of-dict-of-callable). I'd suggest combining them into a single PR with one recursive normalizer that walks lists, tuples, and dicts uniformly and converts callables (and objects with .serialize()) at any depth. Roughly:

def _normalize(obj):
    if isinstance(obj, dict):
        return {k: _normalize(v) for k, v in sorted(obj.items(), key=lambda kv: (type(kv[0]).__name__, repr(kv[0])))}
    if isinstance(obj, (list, tuple)):
        return [_normalize(item) for item in obj]
    try:
        return obj.serialize()
    except AttributeError:
        pass
    if callable(obj):
        return f"<callable {qualname(obj, True)}>"
    return obj

One pass, one helper, both bugs fixed. Also avoids the sorted() mixed-key crash I flagged on #63871.

@wjddn279
Copy link
Copy Markdown
Contributor Author

@ephraimbuddy

Ok, I'll apply the changes to this PR #63871 at once. I'll mention you again once it's done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dag Version Inflation for Dynamic Task Mapping

4 participants