Giter VIP home page Giter VIP logo

Comments (38)

gazpachoking avatar gazpachoking commented on May 11, 2024

Not exactly sure what should be included if it was allOf that failed. It would be nice to be able to tell which schemas were the failing ones in that case.

from jsonschema.

Julian avatar Julian commented on May 11, 2024

as well as the value for the schema keyword

Is this ValidationError.validator?

And as for the instance, reduce(operator.methodcaller("__getitem__"), error.path, instance) or thereabouts should do it, though sure I wouldn't be opposed to adding that as a property on the error.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

Is this ValidationError.validator?

That just stores the keyword, no? In addition to knowing minItems failed, I'd like to also know that minItems was 3, for example.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

And yes, knowing the path is certainly enough to get the value of the instance back, was just thinking it'd be nice to have directly in the error.

from jsonschema.

Julian avatar Julian commented on May 11, 2024

Ah. So yeah that's schema["minItems"] unless you have to deal with nested schemas or whatever in which case you have to look at path too. So yeah that's probably a good candidate for another property I guess.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

Yeah, it's much tougher to trace the path on the schema side for nested schemas, since a path of a/0/1 might trace to properties/a/items/0/additionalItems

from jsonschema.

Julian avatar Julian commented on May 11, 2024

Indeed. OK, sounds good to me if we can do it reasonably.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

The more complicated case is similar to #51, when the types validator (in draft 3), or the anyOf, oneOf, or not keywords might have child errors that you are interested in. Currently the anyOf validator is the worst in my case, where the default error message of 'The instance is not valid under any of the given schemas.' is not very helpful at all.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

Perhaps we can do something with the casue attribute for these cases. The issue is that there may be more than one cause.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

This is not fully related, but perhaps we could also redo the error class hierarchy such that all of the errors were subclasses of a JsonschemaError. We could handle the message attribute in there, as well as __str__ stuff. This would enable somebody to try validation and catch JsonschemaErrors no matter whether they were from validation, metavalidaton or ref resolution.

from jsonschema.

Julian avatar Julian commented on May 11, 2024

I avoid doing that to avoid encouraging people to be overzealous and catch more than they should. There's nothing wrong with except (ValidationError, SchemaError) if that makes sense (catching those along with much more than that though would be silly most likely).

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

Sounds reasonable. I'll write up something for the simpler cases I mentioned here later tonight, those more complex cases I need to think about what information is most useful and how to best deliver it more.

from jsonschema.

Julian avatar Julian commented on May 11, 2024

Sounds good to me.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

Just trying to collect my thoughts here.

I think you are right that it is easy enough to get at the instance using the path, so perhaps attaching that is not needed. Along those lines, maybe providing something like validator_path would be better than just providing the value given to the validator. The value could be easily looked up in the schema using that path.

As for anyOf/oneOf/not... What I would like, is to be able to get the list of errors from each of the possible subschemas that were validated. How those should be packaged and attach them to the error, I'm still not sure. allOf is currently handled differently, in that the allOf validator will never cause a validation error itself, the subschema errors are already the ones returned. If we come up with a clean solution to package the child errors, maybe that should be converted to use it as well.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

Perhaps if we just have a list of child errors that would be fine. If each of them had the validator path attached to them they could then be separated into the child schemas they came from if desired.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

I'm liking that idea. I'll put something together tomorrow unless you have any objections or suggestions.

from jsonschema.

Julian avatar Julian commented on May 11, 2024

Sounds reasonable as long as we find a nice way to attach that to the errors.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

Hmm. $ref sort of messes up being able to give a tracable validator path.

from jsonschema.

Julian avatar Julian commented on May 11, 2024

Well, we'd need to store the final-reffed schema on the error and then a path within that.

All this bookkeeping though suggests that we might want to have something like a ValidationContext object or something that is responsible for keeping all this information and then finalizing the validation errors as they go out.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

Yeah, might be okay, the issue I have is that if the following schema fails, I'd like to be able to sort out which child errors came from which of the 3 schemas. If the validator path is just relative to the last ref context that information gets lost.

{
    "anyOf": [
        {"$ref": "a"},
        {"$ref": "b"},
        {"$ref": "c"}
    ]
}

Not quite sure how a ValidationContext might work, you just mean we'd set these paths on the context, rather than on the errors themselves?

from jsonschema.

Julian avatar Julian commented on May 11, 2024

All I meant with a context object is that it'd be nice to not shove tons of junk on an exception object. I probably shouldn't have even started with doing that, but whoops! So if we're gonna start with even more detail, it'd be nice if we move towards"this is an error. If you want to know how the hell we got here, here's some other object".

As for how exactly that other object should work to support what you want I haven't thought about yet.

from jsonschema.

Julian avatar Julian commented on May 11, 2024

Essentially I think we are (re) inventing tracebacks, where stack frames are schemas and function calls are ref resolutions and we want a nice way to allow traversing them.

Though I'm not sure if thinking about itlike that helps at all.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

So, if we attach the value for the keyword directly to the error (or context object) I think having the path just contain $ref is fine. The path wouldn't be directly traversable, but you wouldn't really need that if you already have the value.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

Okay, started thinking about this again. Going to try to whip something up after I finish some coursework today. As for having extra details in a context object, you are just thinking another class with all the details which gets attached as an attribute to ValidationError?

from jsonschema.

Julian avatar Julian commented on May 11, 2024

Yeah. Not a big deal, we can work that out after we get the interface down.
On Mar 17, 2013 5:24 PM, "Chase Sterling" [email protected] wrote:

Okay, started thinking about this again. Going to try to whip something up
after I finish some coursework today. As for having extra details in a
context object, you are just thinking another class with all the details
which gets attached as an attribute to ValidationError?


Reply to this email directly or view it on GitHubhttps://github.com//issues/79#issuecomment-15031219
.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

Okay, started a bit of work. I added in a schema_path so far, didn't separate it out into a separate object yet, just put it on ValidationError for now. It ignores refs in the path, as if the json reference object was replaced with the actual schema that it points to. Didn't write any tests yet. I'll work on it more tomorrow.
https://github.com/gazpachoking/jsonschema/tree/validationerror_context

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

I think the schema path is a good thing, I'm no longer too worried about not being able to walk it out without redoing ref resolution. I'm thinking add the validator configuration in there as well, and I'm also tempted to attach the whole subschema. This would mean that all 3 arguments that were passed in to the validate_ method that failed were available on the error. Once all that stuff is in, I think attaching a list of suberrors for validators like anyOf and oneOf(and type for draft 3) will be useful, and they will have enough context to understand where they came from. Thoughts?

from jsonschema.

Julian avatar Julian commented on May 11, 2024

Sounds good to me, I have to either see or work out exactly how many and what attributes will have what in a slightly more complicated case to get a clearer picture but it sounds like we're headed in a reasonable direction if I'm imagining it right.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

I added all the attributes I was thinking of directly to ValidationError for now. Here's what I have:

  • validator_arg
  • instance
  • subschema

Those are the three agruments that get passed into the validate function

  • context

This is a list of errors that caused the current validation error (used in anyOf, oneOf, and draft 3 style type validators)

  • schema_path

This is the path to the validator keyword in the schema that failed. In the case of errors in the context list, the path and schema_path are relative to the path and schema_path of the parent error.

gazpachoking@741c275

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

So, what properties do you think should stay directly in ValidationError other than the message?

from jsonschema.

Julian avatar Julian commented on May 11, 2024

I changed my mind maybe. Not sure I care about separating these off.

I think in the tiny bit I've looked at these seem OK. Not sure I love all the names but I don't have any suggestions yet.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

Yeah, I don't like all the names either. I'll work on some tests in the mean time, let me know if you have any good ideas for better names.

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

Opened a pull for easier tracking of progress on the branch. Added a unit test and fixed the implementation when used in nested schemas.

from jsonschema.

Julian avatar Julian commented on May 11, 2024

Looking good.

Only thing I'm concerned about is keeping it easy for people writing validators to manage this properly. E.g. I think all the attributes you're setting should only be set if they haven't been already. And we need to update the docs.

And maybe just schema instead of subschema is good enough? And validator_value instead of arg?

from jsonschema.

gazpachoking avatar gazpachoking commented on May 11, 2024

Yeah, those names are probably better. Maybe rename validator to validator_keyword too? It seems like that would fit better as a pair with validator_value, although it might not be worth changing what was already there. And I'll fix it up so the conditional is per attribute, although they should all be getting set at the same time, and none of them should be getting set from the validator except for context (and the paths).

from jsonschema.

Julian avatar Julian commented on May 11, 2024

Sounds good to me along with a deprecation warning on a property.

from jsonschema.

Julian avatar Julian commented on May 11, 2024

Maybe giving ValidationError (`_Error) a method to do all this default setting is reasonable.

from jsonschema.

Julian avatar Julian commented on May 11, 2024

Closing this, we're close to finishing the PR so rest of the discussion can go there.

from jsonschema.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.