Skip to content

Add pre-commit hook config#765

Merged
Julian merged 1 commit intopython-jsonschema:mainfrom
AleksaC:pre-commit-hook
Dec 20, 2020
Merged

Add pre-commit hook config#765
Julian merged 1 commit intopython-jsonschema:mainfrom
AleksaC:pre-commit-hook

Conversation

@AleksaC
Copy link
Copy Markdown
Contributor

@AleksaC AleksaC commented Dec 20, 2020

Hi, I think it would be useful to add support for jsonschema as pre-commit hook. I think this would be more convenient than using local hooks or creating a mirror repo.

With the way I set it up, the hook can be used with a config like this:

repos:
  - repo: https://github.com/AleksaC/jsonschema
    rev: 639bc64d82
    hooks:
      - id: jsonschema
        files: |
          (?x)(
            ^valid.json$ |
            ^invalid.json$ |
            ^schema.json$
          )
        args: ["-i", "valid.json", "-i", "invalid.json", "schema.json"]

      - id: jsonschema
        files: |
          (?x)(
            ^valid2.json$ |
            ^invalid2.json$ |
            ^schema2.json$
          )
        args: ["-i", "valid2.json", "-i", "invalid2.json", "schema2.json"]

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 20, 2020

Codecov Report

Merging #765 (639bc64) into main (4003667) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #765   +/-   ##
=======================================
  Coverage   96.37%   96.37%           
=======================================
  Files          17       17           
  Lines        2703     2703           
  Branches      309      309           
=======================================
  Hits         2605     2605           
  Misses         79       79           
  Partials       19       19           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4003667...639bc64. Read the comment docs.

@Julian
Copy link
Copy Markdown
Member

Julian commented Dec 20, 2020

Hi there. Sounds great, thanks! Will merge and see what feedback we get.

@Julian Julian merged commit ebfc217 into python-jsonschema:main Dec 20, 2020
@janosh
Copy link
Copy Markdown

janosh commented Oct 12, 2021

@Julian One piece of feedback (from @asottile, the creator of pre-commit):

this hook is a little bit odd -- it doesn't quite meet the normal invocation structure of pre-commit hooks which accepts [file names as] positional arguments -- I wonder if there's something that can be improved there so the configuration is more easily usable

pre-commit/pre-commit.com#569 (comment)

@Julian
Copy link
Copy Markdown
Member

Julian commented Oct 12, 2021

@janosh Happy to see any improvements -- if I'm honest I've never used jsonschema's own hook myself. If there's a way you see to improve it I'm all for it (otherwise if you don't mind opening a ticket at least to track the feedback I trust Anthony enough to know if something's wrong :)

@janosh
Copy link
Copy Markdown

janosh commented Oct 12, 2021

I'm not actually sure what @asottile is proposing. Presumably, the cleanest CLI interface for pre-commit would be if it accepted the hook file names as positional arguments and the schema file through a flag, i.e.

jsonschema-hook --schema schema.json **/*.json

Not sure, how you would translate between that and jsonschema's current interface though.

@Julian
Copy link
Copy Markdown
Member

Julian commented Oct 12, 2021

I doubt I'll have any time to look at this myself, but that doesn't seem right to me exactly as-is, I doubt the default behavior should be "assume all JSON files in a repo conform to a single schema".

Instead probably someone using the hook should be providing a mapping between schema files and instances to validate under them.

But yeah I'm open to proposals, and yes it seems like a wrapper script meant for pre-commit might be a necessary piece.

@asottile
Copy link
Copy Markdown
Contributor

I think the interface @janosh suggests is right, but the default configuration should still require the user to set up files and args to filter to the targetted files properly and to select the right schema

something like:

-   id: jsonschema-v2
    name: jsonschema
    entry: jsonschema-hook
    language: python
    # require the user to override args to set `[--schema, somefile.json]`
    # require the user to override `files` to filter to the target files
    files: ^$ 

or ~something like that

in this jsonschema-hook would be a script which has the following "interface"

jsonschema-hook --schema SCHEMA [other options] [file [file [...]]]

@AleksaC
Copy link
Copy Markdown
Contributor Author

AleksaC commented Oct 12, 2021

Hi, as the author of this pull request I'd like to chime in. First of all sorry for not giving more thought to this, it worked well for my use case, but I realize that it isn't ideal. I knew that there wasn't a way to use the current cli with pre-commit in idiomatic way, I just assumed that changing the cli for pre-commit wasn't something you'd want to do.

There is still one thing that isn't addressed - if you change the schema you'd probably want to run the hook on all the files it is supposed to check to make sure there aren't any discrepancies. With the wrapper proposed above it would be solved by defining the hook twice, the second definition being something like this:

hooks:
  - id: jsonschema-v2
    files: ^schema.json$
    args: [--schema schema.json, ...files]
    pass_filenames: false

Anyway if you want to add a wrapper for pre-commit, I'd be willing to do that as it's sort of my mess to clean up 😄

@Julian
Copy link
Copy Markdown
Member

Julian commented Oct 12, 2021

@AleksaC it's never a mess, thanks for the original contribution, it wouldn't have happened without you submitting it in the first place!

And indeed I don't want to change the CLI for pre-commit, but a separate (private) wrapper is possibly fine.

It'd be good to have two pre-commit users agree / propose some behavior for the hook -- perhaps you and @janosh, though certainly @asottile gets a big vote if he feels like guiding us further.

I'm just being realistic when I say I probably won't have brainpower to dedicate at the minute, so I can chime in with stuff like the above but likely not help think through what the thing looks like until I see a PR.

@janosh
Copy link
Copy Markdown

janosh commented Oct 12, 2021

@AleksaC I second @Julian, there's nothing to be blamed for. On the contrary, thanks for getting the ball rolling. Happy to help with the wrapper either through PR or comments.

Julian added a commit that referenced this pull request May 3, 2026
c7257e92 refOfUnknownKeyword.json rm unused $id (#890)
bf54c9bd support negative multiples (#888)
e819f329 Fix broken test in v1/ref.json from PR #873
29965c01 Merge pull request #873 from jdesrosiers/fix-v1-meta-schema-ref
3b08f79b test: add some missing edge cases to json-pointer format tests (#877)
a1de34ae test(format): allow consecutive hyphens in hostname (RFC1123) (#874)
aac8a1bd chore: remove duplicate idn-email test and fix ambiguous test descriptions (#881)
78048ead format: add RFC 3339 unknown local offset (-00:00) test for time (#878)
ee3fc3d8 test(draft2020-12): add boundary coverage for maxContains = 0 (#857)
54ed4d1f Add additional URI edge cases and include them in v1 (#850)
87f09d65 Add missing email format edge cases across drafts (#849)
f2720d32 more tests for the duration format: missing units (#875)
e391238a Draft-06/7 don't allow empty enums (#876)
c7fc5095 Add tests for empty enum validation (#870)
20f4897c docs: clarify intent of format-assertion vocabulary test cases (#854)
eb018552 Add annotation tests for $dynamicRef and $dynamicAnchor (#862)
0a1dc5e9 fix: remove duplicate allOf and maxItems entries in draft2020-12 KNOWN set (#865)
06e3102d docs: fix typos and wording in suite documentation (#867)
031e9e9a test(duration): expand RFC 3339 grammar coverage for duration format (#868)
3181cefa Fix issues with v1 tests
06481b14 test: add unicode pattern and patternProperties tests for draft2020-12 (#837)
601aa708 Add exhaustive ABNF-driven tests with RFC 2673 compliance for ipv4 format (#840)
583d7c62 Merge pull request #851 from Anshikakalpana/fix/maximum-test-description-mismatch
61bfe27a fix: apply description fix to v1 maximum test
516d71f7 fix: align test description with actual valid field in maximum.json across drafts
4253477c Merge pull request #836 from VIDIT45AGARWAL/date-time
9e787e38 Add tests for out-of-range time components in date-time format
bce6a47c Merge pull request #793 from 414owen/os/fix-unevaluatedproperties-test-case
a17eb1f6 Merge pull request #833 from JulesGosnell/add-m3-validator
648811b2 Add M3 JSON Schema validator to Who Uses section
32fa6e28 Merge pull request #827 from Shristibot/required-null
086bf9a1 Revert unintended .gitignore change
909b58d7 Remove unrelated files from PR
c957e914 Add boolean and null cases to draft-04 , draft-06 and v1
0650027c Add boolean case to required non-object validation
d8d15620 Reset README to upstream to avoid unrelated changes
f6523449 Add required + null test case for issue #821
75995a1c Merge pull request #818 from fperrad/lua-schema
7e6b3a15 Add lua-schema in README.md
8b826d6b Merge pull request #816 from kpavlov/kotlinx-schema-link
b3ae21c2 Update README.md
646f11da Merge pull request #812 from Shristibot/clarification-on-test-suite
ba3f9505 Update README.md
3fc54179 Merge pull request #808 from json-schema-org/ether/more-fix-remotes
3550ca73 clarify optional tests and document integer example
fac4f6f6 Clarify that the test-suite is not  astyle guide
ece5bd10 Merge pull request #804 from Shristibot/add-if-then-else-tests
525adc90 move aside remotes that are not valid in earlier drafts
bbe22f88 remove redundant remotes that also exist in draft-specific directories
ea0baf8d these files are no longer used, and contain an unresolveable $ref on earlier drafts
bef1e2ed Remove redundant then/else false test cases
910af732 Merge pull request #803 from json-schema-org/gregsdennis/proposals
23ff2750 Fix incorrect test condition, simplify test, and copy tests to all drafts as requested
ccfb91af Fix description style and add false-handling tests for issue #767
bc1763ed Fix description formatting in false-handling tests
bd13fcf7 Add if/then/else test cases for issue #767
dc659514 added documentation about folder in root readme and folder readme
c4a99e00 move propertyDependencies tests to proposals folder
d69537ac Merge pull request #802 from json-schema-org/ether/draft4-boolean-schema
79ee846d Merge pull request #801 from json-schema-org/ether/missing-not-tests
2da8f9a6 remove unused remotes files
15cc161f add missing not tests from #714
47ff61c2 Merge pull request #799 from frawa/frawa-patch-1
4a640206 Merge pull request #800 from frawa/frawa-patch-2
fdabd3f9 Merge pull request #795 from anmolbhadoriya5849/fix/issue-768-additionalItems-case
cd02dece Add Unison section to README
4da52d58 Remove 'typed-json' link from README
232f0795 Revert removal of additionalProperties applicator tests in v1
b4032b96 Revert removal of additionalProperties applicator tests in draft2020-12
df833be4 Remove test case for additionalProperties behavior
0d54c828 Remove test for additionalProperties behavior
756eebd6 Remove valid case for additionalItems validation
a3feff79 Remove valid case for additionalItems validation
8ba96359 Remove test for additionalItems applicators
f3488882 Remove valid test case for additionalItems
da4a5684 Remove valid case for additionalItems validation
a247442b Merge pull request #794 from json-schema-org/ether/dynamic-scopes-subschemas
19810ec1 Add a test for respecting dynamic scopes while avoiding the root of each schema
e0e431f8 Fix unevaluatedProperties & additionalProperties test case
8da0a7bc Merge pull request #792 from jdesrosiers/more-v1-transistion
94c0b0e1 Fix more "draft/next" occurances that we missed
8fd4fcec Merge pull request #786 from jdesrosiers/a-label-tests
be58fa98 Merge pull request #791 from json-schema-org/ether/fix-draft-next-removal
acaece38 stop using the term "draft" to mean "version", and fix the remaining mentions of draft-next (now v1)
980e1052 Merge pull request #790 from dylankerr-bis/add-test-sibling-nested-ref-id
b5037ed7 Add test for sibling $ref and $id in nested schema to v1 suite
b8bb8585 Add more comprehensive tessts for A-labels in the "hostname" format
9b8ef536 Cleanup hostname tests (remove dups, fix descriptions, reorder)
4cf55996 Merge pull request #776 from davishmcclurg/idn-hostname-separators
a930db43 Shorten test descriptions to appease ci
aab08752 Updates form PR feedback
081a16a7 Test IDN label separators separate labels
658c8cf9 Test IDN label separator in `hostname` format
38c04b83 Test label separator position in hostname formats
cabbd65c Merge pull request #735 from zaplapl/main
ed74ee74 Add unicode tests to all the dialects
fd0df6cf Update unicode tests per PR feedback
60098220 base test implementation for const equality for strings based on evalutaion by individual codepoint
ef78a0eb Merge pull request #683 from mwadams/main
db365735 Added tests to encoded refs to unknown keywords.
59944b7e Add test for sibling $ref and $id in nested schema to 2019 suite
2196a531 Add test for sibling $ref and $id in nested schema to 2020 suite
e99b24c9 Merge pull request #788 from jdesrosiers/next-to-v1
7eac79be Bring tests up-to-date with format changes
ae27be13 Bring dynamic reference tests up to date with spec changes
79dc2fd3 Unknown optional vocabs can't be ignored anymore
51f84640 Change draft-next to v1
f4e41b0c Merge pull request #785 from jdesrosiers/uneval-nested-conflicts
60a2633f Merge pull request #778 from koplas/extend-uri-validation-tests
bf65ef20 Copy uri tests to other drafts
02a0d89f Merge pull request #743 from Vinit-Pandit/MeastroZI-patch-1
c1545e6c Update unevaluatedProperties.json
7e970016 Merge pull request #764 from Dragonsangel/date_time_validation
8970b1ca Add tests for evaluated property/item nesting conflicts
677a9399 Merge pull request #784 from sangamon/781-date-time-format-extended-year
9672a5b6 Merge pull request #783 from sangamon/780-uuid-format-shifted-dashes
28e49b14 add test for invalid "shifted" dashes in uuid format (#780)
aecf038c add test for invalid extended year in date-time format (#781)
15e4505b Merge pull request #765 from JDepooter/unevaluated_items_with_mincontains_0
b8611c40 Extend URI and URI-reference tests
48461fc3 Merge pull request #775 from jviotti/annotations-content-id
9f7dce32 Merge pull request #766 from json-schema-org/ether/bare-email
a1e077d4 ensure we do not parse email strings permissively
72c670b0 Delete identifiers in Content annotation tests
675186ba Take into account `$id` for annotation schema location assertions
100e9823 Add tests for unevaluatedItems with minContains = 0
0be5549b Added tests to ensure date and time formats don't accept date-time formats

git-subtree-dir: json
git-subtree-split: c7257e92580678a086f0b9243a1903ed88bd27f7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants