Expose an exception-raising entry point#32
Merged
akshayjshah merged 2 commits intomainfrom Jul 10, 2023
Merged
Conversation
akshayjshah
commented
Jul 8, 2023
| collect_violations = _validator.collect_violations | ||
|
|
||
| __all__ = ["Validator", "CompilationError", "Violations", "validate"] | ||
| __all__ = ["Validator", "CompilationError", "ValidationError", "Violations", "validate", "collect_violations"] |
Contributor
Author
There was a problem hiding this comment.
I'm on the fence about exposing collect_violations at the top level like this. Depending on what you all think, I'd be okay requiring people to construct their own validator to use that method.
akshayjshah
commented
Jul 8, 2023
| except protovalidate.ValidationError as e: | ||
| assert len(e.errors()) == 1 | ||
| assert len(e.violations.violations) == 1 | ||
| assert str(e) == "invalid MapMinMax" |
Contributor
Author
There was a problem hiding this comment.
This feels more like bog-standard Python to me. What do you all think?
Collaborator
There was a problem hiding this comment.
Looks good, but need to update the main readme as well.
elliotmjackson
approved these changes
Jul 10, 2023
Contributor
elliotmjackson
left a comment
There was a problem hiding this comment.
Im happy to defer to your expertise here Akshay, whilst its different to the UX of other implementations, we want it to feel idiomatic to the language over all else.
`protovalidate.validate` is the main entry point for this package, but it's not particularly Pythonic: it returns a collection of violations rather than raising an exception. This commit splits the package entry point into `validate` and `collect_violations`, shunting most performance and protobuf-specific concerns to `collect_violations`. `validate` is now more similar to other Python validators: it takes a message and raises a `ValidationError` if the message is invalid. The exception exposes the raw protobuf `Violations` object (in case it needs to be sent over the network as an error detail), but it also exposes an `errors()` method that returns a simple Python list of violations. This matches `pydantic`, the most popular validation library in Python, as closely as we can while still separating unmarshalling from validation. `collect_violations` is now the lower-level entry point. It can optionally append violations to a user-provided `Violations`, and it lets the user choose whether and how to raise an exception. To my eye, this makes simple use of this package feel more like normal Python while still permitting more complex use.
da844bf to
ef6df58
Compare
Contributor
Author
|
😳 Completely forgot about that! Will do now.
…On Tue, Jul 11, 2023 at 7:38 AM Alfred Fuller ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tests/validate_test.py
<#32 (comment)>
:
> assert len(violations.violations) == 0
def test_maps():
msg = maps_pb2.MapMinMax()
- violations = protovalidate.validate(msg)
+ try:
+ protovalidate.validate(msg)
+ except protovalidate.ValidationError as e:
+ assert len(e.errors()) == 1
+ assert len(e.violations.violations) == 1
+ assert str(e) == "invalid MapMinMax"
Looks good, but need to update the main readme as well.
—
Reply to this email directly, view it on GitHub
<#32 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHNP5TFKC7TLGP3AYM5EFDXPVQNPANCNFSM6AAAAAA2CYOPNQ>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
stefanvanburen
added a commit
that referenced
this pull request
Aug 26, 2025
At risk of Chesterton's fencing myself, but I don't think this argument is needed or ever used (other than in tests). I can't think of a reason why someone would want to shove multiple validations into the same validation, instead of simply calling `collect_violations` multiple times and combining the lists afterwards. I did some `git blame` spelunking and it looks like this originally turned up in #32, but didn't see much reasoning for it in that PR. The PR mentioned both Pydantic and making things closer to protovalidate-go's API, but neither seem to have a similar style of reusing a returned error/exception like this. Stacked on #364 as these both munge with the exported API & docstrings; will tag a `v0.15.0` release once all of this is merged.
stefanvanburen
added a commit
that referenced
this pull request
Aug 27, 2025
At risk of Chesterton's fencing myself, but I don't think this argument is needed or ever used (other than in tests). I can't think of a reason why someone would want to shove multiple validations into the same validation, instead of simply calling `collect_violations` multiple times and combining the lists afterwards. I did some `git blame` spelunking and it looks like this originally turned up in #32, but didn't see much reasoning for it in that PR. The PR mentioned both Pydantic and making things closer to protovalidate-go's API, but neither seem to have a similar style of reusing a returned error/exception like this. Stacked on #364 as these both munge with the exported API & docstrings; will tag a `v0.15.0` release once all of this is merged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
protovalidate.validateis the main entry point for this package, but it's not particularly Pythonic: it returns a collection of violations rather than raising an exception. This commit splits the package entry point intovalidateandcollect_violations, shunting most performance and protobuf-specific concerns tocollect_violations.validateis now more similar to other Python validators: it takes a message and raises aValidationErrorif the message is invalid. The exception exposes the raw protobufViolationsobject (in case it needs to be sent over the network as an error detail), but it also exposes anerrors()method that returns a simple Python list of violations. This matchespydantic, the most popular validation library in Python, as closely as we can without controlling the protobuf-generated initializers.collect_violationsis now the lower-level entry point. It can optionally append violations to a user-providedViolations, and it lets the user choose whether and how to raise an exception.To my eye, this makes simple use of this package feel more like normal Python while still permitting more complex use.