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

Fix to schema generation for IPv{4,6}{Address,Interface,Network} #532

Merged
merged 5 commits into from May 21, 2019

Conversation

Projects
None yet
3 participants
@euri10
Copy link
Contributor

commented May 20, 2019

Change Summary

Added schema generation for all IPv{4,6}{Address,Interface,Network}, inspired by #498

The only thing modified that is of interest, and I'm not too sure about, is the order in field_class_to_schema_enum_disabled

Should have I kept the original order (Address > Interface > Network) then both test_ipv4interface_type and test_ipv6interface_type would fail with the following:

test_ipv4interface_type

E         Full diff:
E         - {'properties': {'ip_interface': {'format': 'ipv4address',
E         ?                                                  ^^^ --
E         + {'properties': {'ip_interface': {'format': 'ipv4interface',
E         ?                                                 ++++++ ^

With the order Network>Interface>Address all tests pass

Related issue number

complements #498

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • HISTORY.rst has been updated
    • if this is the first change since a release, please add a new section
    • include the issue number or this pull request number #<number>
    • include your github username @<whomever>
@codecov

This comment has been minimized.

Copy link

commented May 20, 2019

Codecov Report

Merging #532 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master   #532   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          14     14           
  Lines        2271   2271           
  Branches      447    447           
=====================================
  Hits         2271   2271
@samuelcolvin

This comment has been minimized.

Copy link
Owner

commented May 20, 2019

I think you might also need to update the documentation.

@euri10

This comment has been minimized.

Copy link
Contributor Author

commented May 20, 2019

I think you might also need to update the documentation.

Done, hope I didn't miss another file to update ;)

@tiangolo
Copy link
Collaborator

left a comment

Nice job @euri10 !

I just added a note to myself, in case they include an official format for IP Addresses in the JSON Schema spec, we should update it here.

But before that, this works well.

@samuelcolvin
Copy link
Owner

left a comment

otherwise LGTM.

HISTORY.rst Outdated
@@ -10,6 +10,7 @@ v0.26 (unreleased)
* fix return type hint for ``create_model``, #526 by @dmontagu
* **Breaking Change:** fix ``.dict(skip_keys=True)`` skipping values set via alias (this involves changing
``validate_model()`` to always returns ``Tuple[Dict[str, Any], Set[str], Optional[ValidationError]]``), #517 by @sommd
* fix to schema generation for ``IPv4Address``, ``IPv6Address``, ``IPv4Interface``, ``IPv6Interface``, ``IPv4Network``, ``IPv6Network`` by @euri10

This comment has been minimized.

Copy link
@samuelcolvin

samuelcolvin May 20, 2019

Owner

can you add the PR number here like other changes.

This comment has been minimized.

Copy link
@euri10

euri10 May 20, 2019

Author Contributor

done

@tiangolo
Copy link
Collaborator

left a comment

I just realized there's an official "format" for IP v4 and v6 addresses. Can you please update them @euri10 ?

Show resolved Hide resolved pydantic/schema.py Outdated
Show resolved Hide resolved docs/schema_mapping.py
Show resolved Hide resolved docs/schema_mapping.py
@euri10

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

I just realized there's an official "format" for IP v4 and v6 addresses. Can you please update them @euri10 ?

Just did it, 🦅 eyes !

@tiangolo

This comment has been minimized.

Copy link
Collaborator

commented May 21, 2019

Great, thanks @euri10 !

LGTM

@samuelcolvin samuelcolvin merged commit c0298a8 into samuelcolvin:master May 21, 2019

4 of 7 checks passed

Header rules No header rules processed
Details
Pages changed 1 new file uploaded
Details
Redirect rules No redirect rules processed
Details
Mixed content No mixed content detected
Details
codecov/project 100% remains the same compared to cb2abb1
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
@samuelcolvin

This comment has been minimized.

Copy link
Owner

commented May 21, 2019

awesome. Thank you both.

gangefors added a commit to gangefors/pydantic that referenced this pull request May 31, 2019

Merge remote-tracking branch 'upstream/master'
* upstream/master: (138 commits)
  add 'none-any.whl' to pypi upload (samuelcolvin#564)
  uprev
  update benchmarks (samuelcolvin#563)
  cython (samuelcolvin#548)
  Fix issue with unspecified generic type (samuelcolvin#554)
  Run dataclass' original __post_init__ before validation (samuelcolvin#560)
  try to stop annoying warnings in azure pipeline (samuelcolvin#549)
  azure pipeline failOnStderr: false
  Azure Pipelines - tests for windows (samuelcolvin#538)
  Fix JSON Schema for list, tuple, and set, improving interoperability (samuelcolvin#540)
  uprev.
  Colors (samuelcolvin#516)
  Fix to schema generation for IPv{4,6}{Address,Interface,Network} (samuelcolvin#532)
  Fix __fields_set__ not using alias field names (samuelcolvin#517) (samuelcolvin#518)
  Change return type hint for create_model (samuelcolvin#526)
  Tuple ellipsis (samuelcolvin#512)
  Fix to schema generation for IPvAny{Address,Interface,Network} (samuelcolvin#498) (samuelcolvin#510)
  uprev
  Scheduled monthly dependency update for May (samuelcolvin#499)
  Implement const keyword in Schema. (samuelcolvin#469)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.