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

Breaking change in jsonschema==3.1.0 for Python 2.7 #611

Closed
logachev opened this issue Oct 9, 2019 · 12 comments
Closed

Breaking change in jsonschema==3.1.0 for Python 2.7 #611

logachev opened this issue Oct 9, 2019 · 12 comments
Labels
Bug Something doesn't work the way it should.

Comments

@logachev
Copy link

logachev commented Oct 9, 2019

Recent update 3.1.0 introduces a breaking change for Python 2.7.

Here is a code sample that works with 3.0.0 but is broken with 3.1.0 because of the switch from re to js_regex: #609

from jsonschema import Draft4Validator as Validator
schema = {'type': 'string', 'pattern': r'\d+'}
validator = Validator(schema)
list(validator.iter_errors('123'))

Thrown exception:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/site-packages/jsonschema/validators.py", line 328, in iter_errors
    for error in errors:
  File "/usr/local/lib/python2.7/site-packages/jsonschema/_validators.py", line 200, in pattern
    not js_regex.compile(patrn).search(instance)
  File "/usr/local/lib/python2.7/site-packages/js_regex/_impl.py", line 39, in compile
    raise TypeError("pattern={!r} must be a unicode string".format(pattern))
TypeError: pattern='\\d+' must be a unicode string
@Julian Julian added the Bug Something doesn't work the way it should. label Oct 9, 2019
@Julian
Copy link
Member

Julian commented Oct 9, 2019

Having a look...

@Julian
Copy link
Member

Julian commented Oct 9, 2019

CC @Zac-HD.

@Julian
Copy link
Member

Julian commented Oct 9, 2019

OK, I think I don't see how to fix this yet, seems likely that it needs to happen downstream in js-regex, but I at least see how this slipped by, which is the changes to test_validators here: https://github.com/Julian/jsonschema/pull/609/files#diff-523a4ebdb045b5a1613d6711add86c7fR852

Not sure how I missed that during the review, but pretty sure that modification to the tests is our culprit.

@Julian
Copy link
Member

Julian commented Oct 9, 2019

OK and now I have some idea of what the issue is :( :(, which seems to me yeah downstream, and is due to the ever-hazardous from __future__ import unicode_literals here plus here.

@Julian
Copy link
Member

Julian commented Oct 9, 2019

OK, given that there's 2 separate issues here (#612 and #611) and that this is fairly fundamental functionality, I've temporarily pushed out v3.1.1 reverting the switchover.

We'll add some tests for the two cases here (and anything else we missed), and then there likely will be a v3.1.2 that goes back again to js-regex again.

@Julian
Copy link
Member

Julian commented Oct 9, 2019

Looking back at the test changes, not really good that those were mixed in with a rather unrelated test case, it's unsurprising that it wasn't obvious not to change them... so probably this should have been tested much more explicitly anyhow. Will need to add tests specifically here on passing bytes in.

@Zac-HD
Copy link
Member

Zac-HD commented Oct 10, 2019

Just to check, do we want to support bytes on Python 3? My understanding was that JS was always at least Unicode BMP, which would not fit in the bytes type...

@Julian
Copy link
Member

Julian commented Oct 10, 2019 via email

@Julian
Copy link
Member

Julian commented Oct 11, 2019

@Zac-HD I think 204c0c6 covers us for this one.

@logachev
Copy link
Author

Thanks @Julian for a quick mitigation!

@Julian
Copy link
Member

Julian commented Oct 16, 2019

Don't fully thank me until we're re-released :)

(And then thank @Zac-HD for helping contribute something I had 0 desire to ever get into, so real props for that as soon as we get this out the door :)

@Zac-HD
Copy link
Member

Zac-HD commented Oct 17, 2019

This is fixed in js-regex == 1.0.1 🎉

(the metacharacters-in-sets thing is trickier, but in progress)

@Julian Julian closed this as completed Feb 1, 2020
Julian added a commit that referenced this issue Nov 28, 2022
78c888273 Merge pull request #618 from json-schema-org/gregsdennis/contentschema-should-fail-content
5cbc53bc0 Merge pull request #613 from santhosh-tekuri/rjp-multidigit
bb000ce98 Merge pull request #620 from json-schema-org/ether/remove-unused-remotes
c4c490f1d Merge pull request #617 from json-schema-org/gregsdennis/dynamicAnchor-inside-propertyDependencies
0189831a9 remove schemas that are never referenced
cf1b94249 Merge pull request #610 from handrews/rm-remotes
d6490e817 move anchors into defs
2f9d117c0 Merge pull request #616 from json-schema-org/gregsdennis/propertyDependencies-and-unevaluated
4e5649cd0 move tests to draft-next
a41f2f6c4 added type:object to contentSchema schemas
2f50e7864 add tests for $dynamicAnchor in multiple branches of propertyDependencies
4794a1991 add tests for unevaluatedProperties seeing inside propertyDependencies
27cc299f3 Add RJP test 'multi-digit integer prefix'
716b95d94 Merge pull request #612 from santhosh-tekuri/rjp-positive
320c804d1 Add RJP test 'explicit positive prefix'
c8f210c39 Merge pull request #611 from santhosh-tekuri/time-alpha
3faeb222d add time test 'contains letters'
642441f2d Merge pull request #603 from santhosh-tekuri/uuid-nonstr
94d5043c7 add non-string uri tests
0c81374a2 Remove unneeded remotes
97a3e2156 Merge pull request #608 from json-schema-org/json-everything-uses-the-test-suite
bdaf7e8be added json-everything to 'who uses' section of readme; removed manatee.json (deprecated)
f00ec1008 Merge pull request #606 from santhosh-tekuri/duration-nounit
134480721 Merge pull request #607 from santhosh-tekuri/time-offsetprefix
dd4538eee Test time format 'offset not starting with plus or minus'
80fe2db15 test duration format 'element without unit'
38ea15116 Merge pull request #604 from santhosh-tekuri/time-offset
613ec170e second fraction, not offset
ee4bd4eb6 Add time format test with second fraction, no offset
86c2517cd Merge pull request #605 from santhosh-tekuri/rjp-empty
cfe80006a Add relative-json-pointer test with empty string
31796b3b8 add non-string uuid tests

git-subtree-dir: json
git-subtree-split: 78c8882732bcdc2dad81cd7ce1e3f9bca6fb7a9d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something doesn't work the way it should.
Projects
None yet
Development

No branches or pull requests

3 participants