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

Added validation for default values #324

Merged
merged 1 commit into from
Feb 3, 2018

Conversation

bob1de
Copy link
Contributor

@bob1de bob1de commented Feb 3, 2018

Hi,

This PR adds validation for default values as discussed with @alecthomas in #264.

I tested the changes locally and got the expected behaviour so far, unittests passed as well.

It has to be mentioned that this breaks backwards compatibility if one used default values before which would now fail validation, but doing so should be considered as bad practice IMHO anyway. So this change should encourage to not insert default values that violate ones own schema.

Best regards
Robert

@coveralls
Copy link

coveralls commented Feb 3, 2018

Coverage Status

Coverage decreased (-0.004%) to 95.256% when pulling 9f91655 on efficiosoft:validate_defaults into d27d1ee on alecthomas:master.

@bob1de
Copy link
Contributor Author

bob1de commented Feb 3, 2018

Don't know what exactly caused the travis build to fail, but it seems not related to my changes. At least I hope so :-).

error = None
errors = []
for key, value in iterable:
for key, value in key_value_map.items():
Copy link
Owner

Choose a reason for hiding this comment

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

I'm pretty sure this line is what's causing the test failure. key_value_map is not an OrderedDict so it fails the ordering guarantee.

@bob1de
Copy link
Contributor Author

bob1de commented Feb 3, 2018

Oh, I haven't looked deeper into that. Is there a reason you want to guarantee ordering? Since it's just key/value validation order shouldn't matter, should it?

@alecthomas
Copy link
Owner

alecthomas commented Feb 3, 2018

If the user is validating an OrderedDict then the schema validator should maintain key order. You should just be able to replace key_value_map = {} with key_value_map = out.__class__() to achieve this.

@bob1de
Copy link
Contributor Author

bob1de commented Feb 3, 2018

Cool, that made it pass through.

@alecthomas alecthomas merged commit aa29b08 into alecthomas:master Feb 3, 2018
@alecthomas
Copy link
Owner

Thanks!

@bob1de
Copy link
Contributor Author

bob1de commented Feb 4, 2018

You're welcome. Is there already a schedule for the release this is going to be included in? As it will break backwards compatibility with badly written schemas I suppose it will going to be 0.11.0...

@bob1de bob1de deleted the validate_defaults branch February 4, 2018 11:46
svisser added a commit to svisser/voluptuous that referenced this pull request Feb 9, 2018
The change in pull request alecthomas#324 can be backward incompatible if a default value does not validate against the schema
alecthomas pushed a commit that referenced this pull request Feb 9, 2018
The change in pull request #324 can be backward incompatible if a default value does not validate against the schema
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Aug 13, 2019
Upstream changes are many minor improvements and bugfixes, plus

**Changes**

- [#378](alecthomas/voluptuous#378): Allow `extend()` of a `Schema` to return a subclass of a `Schema` as well.

**Changes**:

- [#349](alecthomas/voluptuous#349): Support Python 3.7.
- [#343](alecthomas/voluptuous#343): Drop support for Python 3.3.

**Changes**:

- [#293](alecthomas/voluptuous#293): Support Python 3.6.
- [#294](alecthomas/voluptuous#294): Drop support for Python 2.6, 3.1 and 3.2.
- [#318](alecthomas/voluptuous#318): Allow to use nested schema and allow any validator to be compiled.
- [#324](alecthomas/voluptuous#324):
  Default values MUST now pass validation just as any regular value. This is a backward incompatible change if a schema uses default values that don't pass validation against the specified schema.
- [#328](alecthomas/voluptuous#328):
  Modify `__lt__` in Marker class to allow comparison with non Marker objects, such as str and int.
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.

3 participants