non GPL format option #619
Conversation
|
Hi, thanks a lot for trying to tackle this! I'm definitely still open to the idea of having non-GPL implementations available. On the other hand though I don't want to maintain those here in this library -- (I actually don't want to maintain any format related code :) GPL or otherwise). Meaning -- if you want to create libraries that essentially contain each of these pieces of functionality, and promise to maintain them (or obviously find existing such libraries :) then I'm happy to depend on those, but yeah I can't be maintaining huge monstrous regexes here within this library. Hopefully that makes sense enough, but happy to hear your thoughts. |
|
Hi, Julian! Thank you for your prompt response. It makes sense to me. I will create two separate libraries (validate-rfc3339 and validate-rfc3986) and refactor the PR code to use both libraries. In fact, when I was developing the validators I created them as two different projects with their UT and all, so I think it will be the best approach. |
|
If you have any other suggestions I am eager to hear them. |
|
Cool! Let me have a more careful review (likely will be tomorrow) and will probably have a few other comments yeah. And thanks again! |
|
Hi again, @Julian ! |
There was a problem hiding this comment.
Cool, well done!
I assume you're willing to also maintain those going forward hopefully right (i.e. triage issues)? (With no hard commitment, it's open source hooray...)
Also -- did you happen to check all the transitive dependencies (of everything) to ensure the non-GPL one really doesn't pull in any non-GPL code beyond just the direct format dependencies?
The only other thing left is probably just modifying the section of the docs on format: https://python-jsonschema.readthedocs.io/en/stable/validate/#jsonschema.FormatError
Do you think you could take a stab at adding a sentence or two there about the bare vs. non-GPL options, and modifying that table?
I have a sneaky suspicion we maybe should mention something like "the non-GPL extra is intended to not install any direct dependencies that are non-GPL [but that of course end-users should do their own verification], and that this may mean some formats are unavailable" or some such, whereas the bare extra will install as much functionality as is available anywhere.
Yes, I am willing to also maintain these libs. No problem with that at all.
I've checked and the only two dependencies with MIT incompatible licenses are rfc3987 and strict_rfc3339. I have also solved the style issues and added some words about these changes in the docs. |
Codecov Report
@@ Coverage Diff @@
## master #619 +/- ##
=========================================
- Coverage 96.16% 95.5% -0.67%
=========================================
Files 18 18
Lines 2451 2468 +17
Branches 306 309 +3
=========================================
Hits 2357 2357
- Misses 79 95 +16
- Partials 15 16 +1 |
|
About the coverage reports, I don't know exactly what could we do to test both format and format-nongpl at the same time. Maybe using some environment variables to set the lib preference for the checkers could do the trick to run the unit tests for the gpl and the non-gpl code together. |
|
Thanks! Awesome, think this probably is ready to merge, gonna take one last look first. The coverage thing -- yeah that's something that's "broken" even before this, e.g. we should really be combining coverage across Py2+Py3 too to get the right unioned number, so I don't think that needs to hold this change specifically back, you're right, really we should union the format+formatNonGPL extras probably too. |
|
Merged! Thanks again. |
|
Glad to help! |
|
@naimetti thanks this is really useful to have this is enough to include the patches:
|
|
Probably in the next 4-5 days yeah. |
6b3cac42b Merge pull request #624 from json-schema-org/gregsdennis/propertyDependencies-fixups ef6fb9981 update unevaluatedProperties test to define foo with properties 19130148b add definition for expectedTypes 197cb33ad fixed propertyDependencies $schema & $id; removed invalid test cases a390e321b Merge pull request #619 from json-schema-org/gregsdennis/output-tests b538fe75a Bump the validator version used for suite sanity checks. c26440107 Blacked. 8ee43236d add some more detail in readme; add redundant keyword to some tests c88355294 absoluteKeywordLocation is not required when there is no `$ref` 1a860cfb2 added $id to all output schemas; reindexed test cases to 0 (instead of 1) 930e87e77 add clarification on no changes between 2019 and 2020 f5197e087 Fix the output check to ignore output-schema.json. a8b280571 Minor style tweaks. 3413863a7 Inline the relevant parts of the test schema to output tests. dc6e82046 Merge remote-tracking branch 'origin/main' into gregsdennis/output-tests b1267590c Update bin/jsonschema_suite b40a6cacb Update bin/jsonschema_suite 691d039c1 update $dynamic* value in schemas d263bf014 add blank lines at the end of all the files daf42a748 attempt to update ci for output tests 46bc7ace4 add readme note about output-schema.json files efeee1d4e update tests to reference output schemas 147df4867 add output schemas fe25ba954 updated readme; fixed $schema typo; tests for 2020-12 87c184d20 add .editorconfig; rearrange folders e9f540020 initial run at creating output tests git-subtree-dir: json git-subtree-split: 6b3cac42b95196783d628182061a4e235b109b66
The format extra includes two libs (rfc3987 and strict_rfc3339) that are distributed under GPL licence, which renders jsonschema package to GPL, althought it is licensed under an MIT-like license.
To get rid of these two GPL libs dependencies I have implemented the uri, uri-reference, and date-time validators built-in in the lib, and added the format_nongpl extra to add format validators dependencies without rfc3987 and strict-rfc3339.
An older pull request (#261) suggest using rfc3986 to replace rfc3987, but I have found the RFC3986 validation poorly implemented in that lib.
I have taken the idea to add the new extra in order to maintain backward compatibility from this comment
Docs are still pending upon discussion about this pull request.
Note: the iri and iri-reference validators provided by rfc3987 are not implemented in this pull request.