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

Allow error reporting on keys #479

Merged
merged 5 commits into from
Jun 13, 2023
Merged

Conversation

KenKundert
Copy link
Contributor

I tweaked the error reporting code in voluptuous/schema_builder.py so that validation errors on keys are reported with a message based on the exception raised rather than treated as an extra key. With this change exceptions on keys are treated in the same was as exceptions on values.

Before the change, when an Invalid error is raised on a key, the specifics of the error raised are ignored and the problem was reported as an extra key. This results in several problems. First, the reported error can be misleading. Second, the error could go unreported if the extra parameter is passed as REMOVE_EXTRA or ALLOW_EXTRA to Schema.

One example where this was problematic is a case where I was expecting the keys to be dates, and any invalid dates, such as February 30, should be reported as invalid dates, but instead were reported as an extra key, which is confusing.

Here is a short program that illustrates the problem:

import arrow
from voluptuous import Schema, Invalid, MultipleInvalid

def as_date(arg):
    return arrow.get(arg, 'D MMMM YYYY')

data = {
    "29 February 2020": "...",
    "29 February 2021": "...",
    "29 February 2022": "...",
    "29 February 2023": "...",
}

validate = Schema({as_date: str})

try:
    validate(data)
except MultipleInvalid as e:
    for error in e.errors:
        print(error)

@alecthomas alecthomas merged commit 41bc53d into alecthomas:master Jun 13, 2023
@alecthomas
Copy link
Owner

Thanks!

Comment on lines +433 to +435
if error:
errors.append(error)
elif remove_key:
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it introduced a regression and vol.Remove doesn't work anymore

I think it needs to be reversed

                    if remove_key:
                        # remove key
                        continue
                    elif error:
                        errors.append(error)

bdraco added a commit to bdraco/voluptuous that referenced this pull request Jun 23, 2024
alecthomas pushed a commit that referenced this pull request Jun 23, 2024
* Fix vol.Remove not removing keys

fixes a regression from #479
blocks #479

* add test
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