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

Collect all errors before throwing them #134

Open
dAnjou opened this issue Mar 10, 2017 · 15 comments
Open

Collect all errors before throwing them #134

dAnjou opened this issue Mar 10, 2017 · 15 comments

Comments

@dAnjou
Copy link

dAnjou commented Mar 10, 2017

Instead of raising an error as soon as possible it would be nice to collect all of them and return or raise them all at once.

@sjortiz
Copy link
Contributor

sjortiz commented Apr 21, 2018

@keleshev @dAnjou I would like to work on this

@skorokithakis
Copy link
Collaborator

Please feel free. The only thing I'd like to see is making the error as user-friendly as possible, as I generally like to pass the error to the users when doing things like validating an API's input data.

@sjortiz
Copy link
Contributor

sjortiz commented Apr 26, 2018

@skorokithakis I was careful looking at this seems like the options are not good:
1 - Save all the exceptions in one list and return it, which will result in something like this:

[{'name': [SchemaError("len('') should evaluate to True",)], 'age': [SchemaError('<lambda>(0) should evaluate to True',)], 'gender': 'squid'}, {'name': 'Sam', 'age': 42}, {'name': 'Sacha', 'age': 20, 'gender': 'kid'}]

Thus this can be checked to return the errors, I still don't like it

2 - Save the text of the exections and give it back, which won't allow the user to use try/catch in their end, nor know if an exception was raised

Anyway, I'm open to any suggestions, bear in mind that it's basically imposible (as far as I know) to raise multiple exceptions from one module.

@mlhamel
Copy link

mlhamel commented Apr 26, 2018

Just my two cents,

Maybe a first step that could be convenient just adding something:

def is_valid(self, data):
    try:
        self.validate(data)
        return True
    except SchemaError:
        return False

Some time you just not care of what happen and you just want to know if it pass. Then there's the question of what should you do if you want to get the errors after the fact.

Collecting the errors and then returning them is not necessarily a bad idea. Although validate became huge enough that the coupling might be difficult to play with.

@skorokithakis
Copy link
Collaborator

Hmm, yes, I can see why you don't like either option. Incidentally, I like @mlhamel's suggestion, as there were many times when I just wanted to know if something validates. If anyone would like to submit a small PR for that, I'd be grateful.

As for the problem at hand, we could go with the first approach and add that as an attribute to the exception that was raised, something like e.errors or similar, but yeah, I'm not a huge fan...

@sjortiz
Copy link
Contributor

sjortiz commented Apr 26, 2018

I'll implement @mlhamel solution today/tomorrow.

The other one (the one in question), yeah we can do e.errors and see if I can add a new type of exception called "multi_errors" (or something alike) that it raises message consist in displaying the values in the array, after that I'll post a sample here, I'm not sure yet but, I think it won't have the traceback.

@mlhamel
Copy link

mlhamel commented Apr 30, 2018

Let me know how it goes, I would be happy to give hand also !

@sjortiz
Copy link
Contributor

sjortiz commented May 8, 2018

Sorry for the delay on this guys.
@mlhamel that's the PR that includes your feature.
@skorokithakis Would be really good if you can review this, as I'm not able to assign reviewers.

@sjortiz
Copy link
Contributor

sjortiz commented May 8, 2018

also I realized the fact that the current building doesn't support type hitting in any python version, might be worth checking/requesting in a new issue if you think it will be useful.
@skorokithakis

skorokithakis pushed a commit that referenced this issue May 8, 2018
* creating is_valid method as suggested in the issue 134

* Remove newlines
@skorokithakis
Copy link
Collaborator

Sorry @sjortiz, which PR are you referring to?

@dlobue
Copy link

dlobue commented Jun 6, 2018

I'd love to see this feature as well!

@vinayak-mehta
Copy link

Are there any plans to implement this? Would love to get all validation errors.

@skorokithakis
Copy link
Collaborator

Not currently, but PRs are definitely accepted.

@bandtank
Copy link

bandtank commented Feb 15, 2020

Any movement on this? It's a fairly big problem imo. To fix a broken schema, you have to iterate manually and that could be untenable if the schema is huge.

@acylam
Copy link

acylam commented Aug 30, 2022

Looking for something to validate multiple fields at once and replace invalid fields with placeholder values. A feature like this would be essential. Maybe as a first step, return a dictionary with the same structure as the input and values as booleans indicating whether that specific field passed? You can also raise a MultiSchemaError and make the validated dict available to e.errors as well

@skorokithakis I was careful looking at this seems like the options are not good: 1 - Save all the exceptions in one list and return it, which will result in something like this:

[{'name': [SchemaError("len('') should evaluate to True",)], 'age': [SchemaError('<lambda>(0) should evaluate to True',)], 'gender': 'squid'}, {'name': 'Sam', 'age': 42}, {'name': 'Sacha', 'age': 20, 'gender': 'kid'}]

Thus this can be checked to return the errors, I still don't like it

2 - Save the text of the exections and give it back, which won't allow the user to use try/catch in their end, nor know if an exception was raised

Anyway, I'm open to any suggestions, bear in mind that it's basically imposible (as far as I know) to raise multiple exceptions from one module.

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

No branches or pull requests

8 participants