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

Make ValidationError Hashable #477

Closed
adksujan opened this issue Oct 17, 2018 · 5 comments
Closed

Make ValidationError Hashable #477

adksujan opened this issue Oct 17, 2018 · 5 comments
Labels
Enhancement Some new desired functionality

Comments

@adksujan
Copy link

cannot log ValidationError as an exception. Making the exceptions hashable would be great to use in standard logging libraries. Any plans to make ValidationError exception hashable before the v3.0 release??

@Julian
Copy link
Member

Julian commented Oct 17, 2018

Hey yeah this is one of the remaining things I'm thinking about before the release.

The issue with making them hashable is that doing so requires an API change, because they've got mutable attributes on them at the minute, so those would need to become immutable ones.

@Julian Julian changed the title can't logging.exception(ValidationError), raises Exception TypeError: unhashable type: 'ValidationError' Make ValidationError Hashable Nov 2, 2018
@Julian Julian added the Enhancement Some new desired functionality label Nov 2, 2018
@stuarteberg
Copy link
Contributor

stuarteberg commented Nov 30, 2018

Of course, ValidationError and SchemaError would be trivially hashable if they didn't define __eq__. Is it necessary for those classes to be comparable? I tried grepping the jsonschema code base for == and !=, and as far as I can tell, exceptions are never explicitly compared. (But I could definitely be missing implicit comparisons, such as if exc in mylist:, etc.)

If comparability is not necessary, I vote for simply removing _Error.__eq__() and _Error.__ne__().

For now, here's my workaround:

from jsonschema.exceptions import _Error
_Error.__eq__ = object.__eq__
_Error.__ne__ = object.__ne__
_Error.__hash__ = object.__hash__

Making the exceptions hashable would be great to use in standard logging libraries.

Here's another use-case: pytest can't handle unhashable exceptions, so any users whose tests might raise a ValidationError will currently see an INTERNALERROR traceback from pytest, crashing the whole test suite.

For example:

# test.py
from jsonschema import validate
def test():
    validate([], {"type": "object"})
$ pytest test.py

...

INTERNALERROR>   File "/prefix/lib/python3.6/site-packages/_pytest/_code/code.py", line 398, in exconly
INTERNALERROR>     lines = format_exception_only(self.type, self.value)
INTERNALERROR>   File "/prefix/lib/python3.6/traceback.py", line 136, in format_exception_only
INTERNALERROR>     return list(TracebackException(etype, value, None).format_exception_only())
INTERNALERROR>   File "/prefix/lib/python3.6/traceback.py", line 462, in __init__
INTERNALERROR>     _seen.add(exc_value)
INTERNALERROR> TypeError: unhashable type: 'ValidationError'

@Julian
Copy link
Member

Julian commented Dec 4, 2018

It's used in tests, yes, via assertEqual.

Here's another use-case: pytest can't handle unhashable exceptions, so any users whose tests might raise a ValidationError will currently see an INTERNALERROR traceback from pytest, crashing the whole test suite.

Sounds like a py.test bug worth filing regardless, though presumably it's just the same underlying one (that py~34-36 broke this, and then fixed it again in 3.7). pytest seems like the sort of thing though that should be trying to fix it even on those though.

@stuarteberg
Copy link
Contributor

It's used in tests, yes, via assertEqual.

Ah, thanks for pointing that out. Still, it's not used in many places, so I've proposed #502 to eliminate the need for comparable exceptions.

Sounds like a py.test bug worth filing regardless

OK, I've submitted pytest-dev/pytest#4514, to see if they're interested in supporting unhashable exceptions.

@stuarteberg
Copy link
Contributor

presumably it's just the same underlying one (that py~34-36 broke this, and then fixed it again in 3.7)

FWIW, you're right: This is not an issue when using Python 3.7. I'm using Python 3.6, where it is an issue.

@Julian Julian closed this as completed in a7ca0c5 Dec 15, 2018
Julian added a commit that referenced this issue Apr 28, 2021
09fd353f Merge pull request #481 from kylef/kylef/time
0ed2e79b Fix negative time test to only fail on a single rule
2edc74b1 Add valid time with different second fractions
7bde0bf7 Add valid time with leap second including offset
ee83f464 Stricter time format constraints
5732904a Merge pull request #480 from json-schema-org/ether/better-test-names
c2994271 better test names for schema-items + additionalItems
6bc53e60 Merge pull request #479 from json-schema-org/fix-non-id-in-enum-for-drafts-6-and-7
3f783d9c fixing draft 6 & 7 non-id tests
5768c68d Merge pull request #476 from json-schema-org/ether/readme-updates
0c8bfc06 add mention of JSON::Schema::Tiny
e4c10c6b fix markdown for underscores in package names
eeb4db18 mention draft2020-12 in readme
dff69dcb Merge pull request #474 from marksparkza/unevaluatedItems-depends-on-contains
51b4977c Merge pull request #478 from sorinsarca/patch-1
dfcd4a19 fix bad comma
4cb100a5 Merge pull request #465 from json-schema-org/ether/more-naive-ref
31dc86bc add another test of naive $ref replacement
f858c613 Merge pull request #477 from json-schema-org/ether/more-items-tests
4e266c34 test that array-items/prefixItems adjusts the starting position for schema-items/additionalItems
b7fced33 Merge pull request #473 from json-schema-org/ether/more-default-tests
eadb9be7 test that a missing property is not populated by the default in the actual instance data
839b95d8 Added opis/json-schema
7cf78800 Add missing comma
3390c871 Update tests/draft2020-12/unevaluatedItems.json
d3b88001 Update tests/draft2020-12/unevaluatedItems.json
84e1d5a9 Add another test case for unevaluatedItems-contains interaction
f400802c Add tests for unevaluatedItems interaction with contains

git-subtree-dir: json
git-subtree-split: 09fd353fc44ab22e7e8998d096b3d6d83287e5e6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Some new desired functionality
Projects
None yet
Development

No branches or pull requests

3 participants