Skip to content

Conversation

@ryyakobe
Copy link
Contributor

The parameter definition validators were failing when allowedValues was explicitly set to None, despite None being a valid value according to the model definition. This fix ensures both explicit None values and omitted allowedValues fields are handled correctly in validation.

Also, fixed a few minor typos and inconsistencies.

Fixes: #179

What was the problem/requirement? (What/Why)

The parameter definition validators were failing with 'NoneType' object is not iterable when allowedValues was explicitly set to None, even though None is a valid value according to the model definition. The validators needed to handle both cases: when allowedValues is omitted (implicitly None) and when it's explicitly set to None.

What was the solution? (How)

Updated the validators in JobStringParameterDefinition, JobPathParameterDefinition, JobIntParameterDefinition, and JobFloatParameterDefinition to:

  1. Handle None values explicitly by returning None early in the validation
  2. Make error locations consistent by using ("allowedValues", i) format
  3. Fix error message in JobIntParameterDefinition from "larger than minValue" to "larger than maxValue"

What is the impact of this change?

This change allows users to explicitly set allowedValues to None in their parameter definitions, which was previously causing validation errors. It also makes error messages more consistent and accurate across all parameter types.

How was this change tested?

The existing test suite covers both cases of omitted allowedValues and explicitly set None values. The tests pass successfully.

  • Have you run the unit tests?
    Yes, all unit tests pass.

Was this change documented?

  • Are relevant docstrings in the code base updated?
    No documentation changes were needed as this fix aligns the implementation with the existing model definition and documentation.

Is this a breaking change?

No, this is not a breaking change. It fixes the implementation to match the intended behavior as defined by the model's type hints and existing documentation.

Does this change impact security?

  • Does the change need to be threat modeled?
    No.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@sonarqubecloud
Copy link

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Parameter validation fails when allowedValues is set to None

2 participants