Skip to content

Replace rfc3987 (GPL) with rfc3986 (Apache) #261

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,8 @@ _static
_templates

TODO
*.pyc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO these all belong in your global gitignore, they're not specific to this project (which is why they weren't here before).

In case you don't already have one here's a sample of mine: https://github.com/Julian/dotfiles/blob/master/.config/git/ignore

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair, I actually didn't know a global gitignore was possible 😄. I will revert those changes

.eggs/
.idea/
.tox/
jsonschema.egg-info/
8 changes: 5 additions & 3 deletions jsonschema/_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,16 +158,18 @@ def is_host_name(instance):


try:
import rfc3987
import rfc3986
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd need to preserve backwards compatibility here.

So we'd need to first try to import rfc3987.parse and then fall back on rfc3986.is_valid_uri. The tests also should be updated to run under both modules (isolated from each other) to ensure that they always both pass the suite. This can be done fairly easily with a tox matrix I think. I can give some more guidance if need be.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, I can manage the code change - but I'll probably need guidance around tox since I'm not familiar with it

except ImportError:
pass
else:
@_checks_drafts("uri", raises=ValueError)
def is_uri(instance):
if not isinstance(instance, str_types):
return True
return rfc3987.parse(instance, rule="URI")

elif not rfc3986.is_valid_uri(instance, require_scheme=True):
return False
else:
return rfc3986.normalize_uri(instance)

try:
import strict_rfc3339
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
]

extras_require = {
"format" : ["rfc3987", "strict-rfc3339", "webcolors"],
"format" : ["rfc3986", "strict-rfc3339", "webcolors"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think probably we'd need to support backwards compat here too. I assume you've already verified that strict-rfc3339 and webcolors are non-GPL?

If so I think we'd need to add a format-nongpl extra, otherwise we'll potentially change the deps being installed for people.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't think of checking the other deps... strict-rfc3339 is GPL as well so we'll need to find an alternative there, webcolors is BSD.

Totally reasonable for the backwards compatibility perspective.

":python_version=='2.6'": ["argparse", "repoze.lru"],
":python_version=='2.7'": ["functools32"],
}
Expand Down