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

feat: Support custom JSON schema validation and string format checkers in targets #1471

Closed
wants to merge 7 commits into from

Conversation

edgarrmondragon
Copy link
Collaborator

@edgarrmondragon edgarrmondragon commented Mar 1, 2023

@codecov
Copy link

codecov bot commented Mar 1, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (dfcec5f) 85.05% compared to head (33b43a1) 87.94%.
Report is 276 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1471      +/-   ##
==========================================
+ Coverage   85.05%   87.94%   +2.88%     
==========================================
  Files          58       59       +1     
  Lines        4845     7417    +2572     
  Branches      805     1642     +837     
==========================================
+ Hits         4121     6523    +2402     
- Misses        526      642     +116     
- Partials      198      252      +54     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@edgarrmondragon edgarrmondragon changed the title feat: Support custom JSON string format checkers in targets feat: Support custom JSON schema validation and string format checkers in targets Mar 1, 2023
@edgarrmondragon edgarrmondragon force-pushed the feat/custom-format-checkers branch from c8f6b33 to 188a622 Compare March 1, 2023 23:35
@BuzzCutNorman
Copy link
Contributor

@edgarrmondragon I like it so far. No format checking at all to start and then you can add checks as you need them. When working with this I added a default format checker that I knew would fail date and time. Then created a simple is_allowed function to get past the validation fail to see how the values filed.

    def get_format_checker(self) -> FormatChecker:
        format_checker: FormatChecker = Draft3Validator.FORMAT_CHECKER

        def is_allowed(value):
            return True

        format_checker.checks("time")(is_allowed)
        format_checker.checks("date")(is_allowed)
        return format_checker

@edgarrmondragon edgarrmondragon force-pushed the feat/custom-format-checkers branch from 229306b to 33b43a1 Compare May 4, 2023 00:28
@edgarrmondragon edgarrmondragon marked this pull request as ready for review May 4, 2023 00:46
@edgarrmondragon edgarrmondragon requested review from a team and cjohnhanson as code owners May 4, 2023 00:46
@edgarrmondragon
Copy link
Collaborator Author

Related: #1831

Copy link

codspeed-hq bot commented Nov 27, 2023

CodSpeed Performance Report

Merging #1471 will not alter performance

Comparing feat/custom-format-checkers (33b43a1) with main (b4a7b3e)

Summary

✅ 4 untouched benchmarks

Copy link
Collaborator Author

@edgarrmondragon edgarrmondragon Jan 4, 2024

Choose a reason for hiding this comment

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

@BuzzCutNorman Do you think this is still relevant? The original implementation was rather specific python-jsonschema so I wonder if we should keep it?

Copy link
Contributor

Choose a reason for hiding this comment

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

@edgarrmondragon If you would please clarify if the question of relevance is about test_format_checker.py or the PR in general?

Copy link
Collaborator Author

@edgarrmondragon edgarrmondragon Jan 5, 2024

Choose a reason for hiding this comment

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

@BuzzCutNorman I commented on the file in the wrong PR 😅. I meant #2136 (comment)

@edgarrmondragon edgarrmondragon deleted the feat/custom-format-checkers branch August 9, 2024 21:48
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.

2 participants