-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Decouple Param and ParamsDict from SDK during deserialization
#55111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
7643d52 to
4697efa
Compare
4697efa to
3a62f2a
Compare
3a62f2a to
fc10d9a
Compare
c693102 to
b9e352c
Compare
297cdc4 to
09eff55
Compare
amoghrajesh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nits, but generally looks good.
kaxil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't approve my own PR. Have a couple of comments but lgtm other than that @uranusjr
bugraoz93
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Introduces server-side `SerializedParam` and `SerializedParamsDict` classes to eliminate SDK dependencies during DAG deserialization. This change ensures the scheduler and API server can deserialize DAGs without importing Task SDK modules. Part of apache#52141
uranusjr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rubber stamping for Kaxil’s approval
…ache#55111) Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
…ache#55111) Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
PR apache#55111 introduced SerializedParam to decouple from the Task SDK, but inadvertently broke the API contract by changing `.dump()` to `.resolve()` in the params serialization. This caused the UI to crash because FlexibleForm expects the full param spec with {value, schema, description} but was receiving only the resolved values. The original SDK Param class had a `.dump()` method that returned the full spec, so this restores the API contract rather than creating new behavior. Related: apache#55111
PR apache#55111 introduced SerializedParam to decouple from the Task SDK, but inadvertently broke the API contract by changing `.dump()` to `.resolve()` in the params serialization. This caused the UI to crash because FlexibleForm expects the full param spec with {value, schema, description} but was receiving only the resolved values. The original SDK Param class had a `.dump()` method that returned the full spec, so this restores the API contract rather than creating new behavior. Related: apache#55111
PR apache#55111 introduced SerializedParam to decouple from the Task SDK, but inadvertently broke the API contract by changing `.dump()` to `.resolve()` in the params serialization. This caused the UI to crash because FlexibleForm expects the full param spec with {value, schema, description} but was receiving only the resolved values. The original SDK Param class had a `.dump()` method that returned the full spec, so this restores the API contract rather than creating new behavior. Related: apache#55111
…ache#55111) Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
PR apache#55111 introduced SerializedParam to decouple from the Task SDK, but inadvertently broke the API contract by changing `.dump()` to `.resolve()` in the params serialization. This caused the UI to crash because FlexibleForm expects the full param spec with {value, schema, description} but was receiving only the resolved values. The original SDK Param class had a `.dump()` method that returned the full spec, so this restores the API contract rather than creating new behavior. Related: apache#55111
PR apache#55111 introduced SerializedParam to decouple from the Task SDK, but inadvertently broke the API contract by changing `.dump()` to `.resolve()` in the params serialization. This caused the UI to crash because FlexibleForm expects the full param spec with {value, schema, description} but was receiving only the resolved values. The original SDK Param class had a `.dump()` method that returned the full spec, so this restores the API contract rather than creating new behavior. Related: apache#55111
…ache#55111) Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
Introduces server-side
SerializedParamandSerializedParamsDictclasses to eliminate SDK dependencies during DAG deserialization. This change ensures the scheduler and API server can deserialize DAGs without importing Task SDK modules.Part of #52141
TODO:
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.