Allow exception types to be registered for format functions #76

gazpachoking · 2013-03-02T06:45:36Z

Implementation of idea in #70, let me know what you think.

The one thing I was unsure of, is when an exception is caught, should it still have the default 'foo' is not a 'bar' error message before the message from the caught exception?

These exceptions will be caught and message will be used for the ValidationError

Julian · 2013-03-03T00:19:16Z

jsonschema.py

+            except raises as e:
+                raise FormatError(unicode(e))
+            if not result:
+                raise FormatError("%r is not a %r" % (instance, format))
        return True


This return is no longer necessary since essentially someone calling this just cares if it raises an exception or not.

I'm not sure how much I like the name conforms for this anymore. conforms sounds like it should return a bool. That's why I suggested check do this, and leave conforms to return a bool if check raised a FormatError (even though in our code we'd never need to use conforms anymore).

Julian · 2013-03-03T00:22:15Z

I would like to prepend the default message, yeah, but doing so might affect people who want the message from their registered exception. Actually they might even want more than the message.

Maybe we could catch the exception object and set it as ValidationError.context (or some other name?) which would allow people to get at it if they wanted it. Like that idea?

gazpachoking · 2013-03-03T05:39:04Z

Something like that might be good. Couple questions, would FormatChecker add it to the FormatError, which would then be transferred to the ValidationError? Also, not sure about the name 'context', although my skills at naming thing appropriately are abysmal, and I don't have any better ideas. My last concern was that this would be the only validating method that has this kind of property on the error; that's not necessarily a problem though.

Also, I agree wrt the rename from conforms to check. Do we even need to leave conforms in there anymore after that?

gazpachoking · 2013-03-03T05:40:30Z

Or did you mean transfer the error message to the FormatError, and attach the FormatError to the ValidationError?

Julian · 2013-03-03T05:47:46Z

The former.

As for naming yes naming sucks :/

The reason I said context is because that's what a similar thing is called in Python 3 for chaining exceptions which is kinda similar. http://www.python.org/dev/peps/pep-3134/

Although the other name, .cause, might be even better.

And yes for now unfortunately format would be the only thing that used that, although I don't doubt that eventually we'll be able to think about other uses for attaching arbitrary extra information to a validation error.

We wouldn't need conforms anymore, no, but I guess there's not really a good reason to break backwards compatibility even for something that I doubt anyone is using yet, when we don't have to. It's still a kind of useful function anyhow.

gazpachoking · 2013-03-03T05:51:33Z

I think I like 'cause' a bit better. I'll work up another commit with these changes.

…r in 'cause' attribute

gazpachoking · 2013-03-03T06:51:09Z

Wasn't really sure how to check the error attributes properly in the tests, there may be a better way than I did it.

Julian · 2013-03-03T12:30:36Z

docs/validate.rst

@@ -251,6 +252,8 @@ be enabled by hooking in a format-checking object into an :class:`IValidator`.
        supplied checker.

        :argument str format: the format that the decorated function will check
+        :argument raises: Exception that will be caught and used as the


Can we add a cross-link here so that people know where these end up? Something like:

:argument Exception raises: the exception(s) raised by the decorated function when an invalid instance is found. The exception object will be accessible as the :attr:`ValidationError.cause` attribute of the resulting validation error.

Sounds good.

Julian · 2013-03-03T12:42:56Z

Left some comments.

Also, can we update all the format checkers to use this now? That would get rid of most of the is_whatever functions but meh who cares.

gazpachoking · 2013-03-03T20:59:32Z

Working on switching the format checkers to use this now.

gazpachoking · 2013-03-03T21:11:52Z

So, do we want to get rid of the is_whatever functions where possible? Also, where we need to add extra parameters to whatever we are using for checking, and keep the is_xx function, should it just return the parsed result, or return True still? Or, do we want to also make the checkers require raising an exception if they fail, and not support the False return anymore?

Julian · 2013-03-03T21:21:59Z

Yeah I guess remove where possible.

And I don't care either way on supporting False / True, do whatever's prettier :D.

gazpachoking · 2013-03-03T21:43:06Z

I'm still thinking it's good to include all the formats in the FormatChecker class, should I do that here too? Or make it a separate pull?

Julian · 2013-03-03T21:43:51Z

Fine with me either way if you have it done already.

gazpachoking · 2013-03-03T21:58:03Z

Hmm, my plan was to register them all to the class, then provide appropriate format= argument when creating draft3 and draft4_format_checker. That is going to cause a problem though if the appropriate module isn't available for all of the formats.

Julian · 2013-03-03T21:59:16Z

Better leave it for later then. Not sure it's a good idea to have it silently ignore non-present formatters.

gazpachoking · 2013-03-03T22:01:46Z

Okay, pushed my commit before that one then, seem good? See anything I missed?

Julian · 2013-03-03T22:04:41Z

Removing the methods that disappeared from the docs.

Otherwise think it's good.

gazpachoking · 2013-03-03T22:11:39Z

Hmm, should we just remove the whole bit about the actual methods that do the checks being exposed? Or just remove the ones that aren't there anymore. I guess if we leave it in we need to document what exceptions might be raised for each.

Julian · 2013-03-03T22:12:07Z

Good idea. Yeah let's just get rid of it.

gazpachoking · 2013-03-03T22:14:25Z

Should we still document the checkers that require extra packages to be available in there though? I'm guessing yes.

Julian · 2013-03-03T22:14:56Z

Yes.

gazpachoking · 2013-03-03T22:32:07Z

Updated the docs, created a new issue, #78, for possible changes for built in format checker handling.

Julian · 2013-03-03T22:35:02Z

tests.py

+        with self.assertRaises(FormatError) as cm:
+            checker.check("bar", "foo")
+        # Original exception should be attached to cause attribute
+        self.assertIs(cm.exception.cause, cause)


Forgot to change this to as e: (and above).

After that feel free to merge :).

assertRaises context manager doesn't actually return the exception, it returns the context manager, which has an exception property. (the exception isn't available when entering the context, which is what gets returned to the as name) Unless I am misunderstanding, this is the way it has to be.

Oh, no, it's right now, GitHub was showing me an old diff apparently.

Ahh. Okay, merging.

Allow exception types to be registered for format checker functions

0b657e8 Merge pull request #86 from kylef/patch-1 1bd4151 [README] JSONSchema.swift uses these tests too 8f86716 Revert "Add jon, JSON parser for the fishshell." db9c629 Merge pull request #82 from bucaran/patch-1 875fa42 Add jon, JSON parser for the fishshell. 64c556c Merge pull request #81 from s-panferov/patch-1 43105d4 Add new Rust library to the list aa4d927 Merge pull request #80 from seagreen/implementations 20ef067 Add new haskell implementation. 1274070 Merge pull request #79 from Muscula/json-schema-benchmark 6d8cf45 Merge pull request #78 from JamesNK/patch-1 55c4992 Add json-schema-benchmark to list of users of this test suite 645623d Added Newtonsoft.Json.Schema implementation a7944d1 Merge pull request #76 from Prestaul/patch-1 5729cdf Added skeemas to list of suite users 4600fe3 Make the implementation list a bit less unwieldy now that it's so long (hooray!) 11d6905 Merge remote-tracking branch 'mafintosh/patch-1' into develop 689b80f Merge pull request #74 from bugventure/develop c36f888 Add request-validator as a user of the test suite 41876b1 Update README.md aabcb34 Merge pull request #71 from seagreen/additionalproperties b3d160b Add tests for additionalProperties by itself. git-subtree-dir: json git-subtree-split: 0b657e8b0d21a4099d3ce378ba7208073a9295e6

Allow exception types to be registered for format functions

f56c41e

These exceptions will be caught and message will be used for the ValidationError

Julian reviewed Mar 3, 2013
View reviewed changes

Change errors during format checking to be attached to ValidationErro…

77d64bc

…r in 'cause' attribute

Julian reviewed Mar 3, 2013
View reviewed changes

Cleanups for FormatChecker error catching

2303c17

Convert built in format checkers to provide original exception

3ee6f67

Fix cls_checks args in docs

a561d32

gazpachoking added 2 commits March 3, 2013 17:19

Update docs for format checkers

2dd9c26

Oops

762a507

Julian reviewed Mar 3, 2013
View reviewed changes

gazpachoking added a commit that referenced this pull request Mar 3, 2013

Merge pull request #76 from gazpachoking/format_errors

d74171f

Allow exception types to be registered for format checker functions

gazpachoking merged commit d74171f into python-jsonschema:master Mar 3, 2013

gazpachoking mentioned this pull request Mar 3, 2013

Custom errors from format checkers? #70

Closed

gazpachoking deleted the format_errors branch March 4, 2013 02:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow exception types to be registered for format functions #76

Allow exception types to be registered for format functions #76

gazpachoking commented Mar 2, 2013

Julian Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian Mar 3, 2013

gazpachoking Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian Mar 3, 2013

gazpachoking Mar 3, 2013

Julian Mar 3, 2013

gazpachoking Mar 3, 2013

Allow exception types to be registered for format functions #76

Allow exception types to be registered for format functions #76

Conversation

gazpachoking commented Mar 2, 2013

Julian Mar 3, 2013

Choose a reason for hiding this comment

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian Mar 3, 2013

Choose a reason for hiding this comment

gazpachoking Mar 3, 2013

Choose a reason for hiding this comment

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian commented Mar 3, 2013

gazpachoking commented Mar 3, 2013

Julian Mar 3, 2013

Choose a reason for hiding this comment

gazpachoking Mar 3, 2013

Choose a reason for hiding this comment

Julian Mar 3, 2013

Choose a reason for hiding this comment

gazpachoking Mar 3, 2013

Choose a reason for hiding this comment