jens-scheffler-bosch commented on PR #31301: URL: https://github.com/apache/airflow/pull/31301#issuecomment-1586347886
> I think this is too relaxed for validation. `validate_schedule_and_params` only check for nullable and default values where `validate` did an actual `Param.resolve`. > > With this change, there is no actual type validation performed before bagging a Dag, and we have no import error if we provide a "string" default value for a 'number' param. > > This will then crash the scheduler on the next scheduled run. Thanks @pierrejeambrun for the feedback. I understand that not validating at-all is potentially opening the door too wide for "any" garbage not valid default param. I adjusted to validate a param if either the DAG is scheduled regularly (so default values must be valid) or if a default value is provided - only if no value or `None` is given as default, validation is skipped. So users need to pass parameters to trigger (else validation fails upong trigger, this is also unit-tested already) During the fix I found three other glitches in consistency, so I fixed this alongside, hope this is OK: * 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 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 and upon deserialization a `Param` with no default came out with the default value `<airflow.utils.types.ArgNotSet object at 0x7fb1c6344c40>`. Fixed serialization and now the `ArgNotSet` object is supported to be serialized/deserialized properly. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
