Skip to content

improve error message for missing required props#176

Merged
horejsek merged 1 commit intohorejsek:masterfrom
davidszotten:improve-required-error-message
Jul 22, 2023
Merged

improve error message for missing required props#176
horejsek merged 1 commit intohorejsek:masterfrom
davidszotten:improve-required-error-message

Conversation

@davidszotten
Copy link
Copy Markdown
Contributor

only include _missing properties in the error message for required props

>>> fastjsonschema.validate({'required': ['foo', 'bar']}, {'foo': 1})

before:

JsonSchemaValueException: data must contain ['foo', 'bar'] properties

after:

JsonSchemaValueException: data must contain ['bar'] properties

fixes #175

only include _missing properties in the error message for required props

```
>>> fastjsonschema.validate({'required': ['foo', 'bar']}, {'foo': 1})
```

before:

```
JsonSchemaValueException: data must contain ['foo', 'bar'] properties
```

after:

```
JsonSchemaValueException: data must contain ['bar'] properties
```
@horejsek
Copy link
Copy Markdown
Owner

Thanks for the pull request. I measured this is actually also slightly faster on my computer, which surprised me.

@horejsek horejsek merged commit 97634bd into horejsek:master Jul 22, 2023
@davidszotten
Copy link
Copy Markdown
Contributor Author

Thank you! Any chance of a new release with this in?

@davidszotten
Copy link
Copy Markdown
Contributor Author

Thanks for the pull request. I measured this is actually also slightly faster on my computer, which surprised me.

maybe marginally faster since there is less string formatting to do?

@horejsek
Copy link
Copy Markdown
Owner

I just released 2.18.0 with an additional fix.

In the morning I tested only happy path. That is, if you require ten props and all of them are there, the whole iteration must be done anyway. But I see your version is slower in case some prop is missing. More missing props, faster original solution is (it also depends on the order of the props) - it simply stops with the first missing item without need to compute the whole diff. Considering it is slower only in unhappy cases for specific schemas and only slightly, I say it is OK.

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.

errors for missing required properties

2 participants