Skip to content

Conversation

dAIsySHEng1
Copy link
Contributor

Change Summary

  • Problem: Pydantic throws a KeyError when WithJsonSchema is used with a $ref that defines an external JSON Schema via an https link.
  • Fix: Add try-excepts to skip lookup if it is failing for a link that begins w/ https or http (suggested by Adrian).

Related issue number

fix #9783

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Jul 7, 2024
Copy link

codspeed-hq bot commented Jul 7, 2024

CodSpeed Performance Report

Merging #9863 will not alter performance

Comparing dAIsySHEng1:json-schema-external-link (5a9ec8c) with main (929543c)

Summary

✅ 17 untouched benchmarks

Copy link
Contributor

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

This looks great! I left a few suggestions for a syntax simplification :)

@sydney-runkle sydney-runkle enabled auto-merge (squash) July 18, 2024 18:10
@sydney-runkle sydney-runkle disabled auto-merge July 18, 2024 18:13
Copy link
Contributor

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

@dAIsySHEng1,

Let's add a test. Looks great otherwise!

@reversefold
Copy link

typing.Annotated was introduced in Python 3.9, so we should probably skip this new test in any version lower than that.

@sydney-runkle
Copy link
Contributor

@adriangb, let's touch base on this one early next week as well 👍

@sydney-runkle sydney-runkle added relnotes-change Used for changes to existing functionality which don't have a better categorization. and removed relnotes-fix Used for bugfixes. awaiting author revision awaiting changes from the PR author labels Aug 16, 2024
@sydney-runkle sydney-runkle changed the title Update json_schema.py to Handle Refs w/ Http Links Allow WithJsonSchema to inject $refs w/ http or https links Aug 16, 2024
@sydney-runkle
Copy link
Contributor

@dAIsySHEng1,

Great work here! Apologies on the delayed review. Hope you have a great semester!

@sydney-runkle sydney-runkle enabled auto-merge (squash) August 16, 2024 16:06
@sydney-runkle sydney-runkle merged commit 823d51f into pydantic:main Aug 16, 2024
57 checks passed
Copy link
Contributor

Coverage report

This PR does not seem to contain any modification to coverable code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes-change Used for changes to existing functionality which don't have a better categorization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Referencing an external JSON Schema when generating a JSONSchema fails with a KeyError
3 participants