Skip to content
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

Fix handling of default value and serialization of Param class #33141

Conversation

jscheffl
Copy link
Contributor

@jscheffl jscheffl commented Aug 5, 2023

-- Split PR form #31301 to reduce volume and concerns --

This PR fixes the behavior of the Param class and resolves problems found while implementing #31301. Problems found there:

  • If no default was provided to Param object the ArgNotSet Object was passed out as value when dump() was called - now if no default is provided, this is now masked with None.
  • If None was provided as default the check for has_value returned True which is not really expected.
  • If no default value was provided to Param and the value was initialized to ArgNotSet then during DAG serialization the object in the value was converted to a string representation. Upon de-serialization a Param with no default came out with the default value <airflow.utils.types.ArgNotSet object at 0x7fb1c6344c40> as string. Fixed serialization and now the ArgNotSet object is supported to be serialized/deserialized properly.

Adjusted pytests accordingly to prevent a regression.

@jscheffl jscheffl added kind:bug This is a clearly a bug area:core labels Aug 5, 2023
@jscheffl jscheffl changed the title Bugfix/param without default bad serialization Fix handling of default value and serialization of Param class Aug 5, 2023
@eladkal eladkal added type:bug-fix Changelog: Bug Fixes and removed kind:bug This is a clearly a bug labels Aug 5, 2023
@eladkal eladkal added this to the Airflow 2.7.1 milestone Aug 5, 2023
@hussein-awala hussein-awala self-requested a review August 12, 2023 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants