Migrate to subclassing instead of annotated approach for pydantic url types #10662
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sydney-runkle
changed the title
CodSpeed Performance ReportMerging #10662 will not alter performanceComparing Summary
|
Deploying pydantic-docs with
|
| Latest commit: |
fa3dad5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fd4627a7.pydantic-docs.pages.dev |
| Branch Preview URL: | https://urls-proof-of-concept.pydantic-docs.pages.dev |
sydney-runkle
commented
Oct 18, 2024
| UrlConstraints(allowed_schemes=['nats', 'tls', 'ws', 'wss'], default_host='localhost', default_port=4222), | ||
| ] | ||
| """A type that will accept any NATS DSN. | ||
| _constraints = UrlConstraints(allowed_schemes=['amqp', 'amqps']) |
There was a problem hiding this comment.
https://www.rabbitmq.com/docs/uri-spec
The host is "required" but can be zero in length... we previously didn't enforce host_required here, so leaving as is, but adding not to the docs.
sydney-runkle
commented
Oct 18, 2024
| @@ -107,7 +107,6 @@ | |||
| 'http://example.org/path#fragment', | |||
| 'http://example.org/path?query#', | |||
| 'http://example.org/path?query#fragment', | |||
| 'file://localhost/foo/bar', | |||
There was a problem hiding this comment.
I don't think this is a valid AnyUrl because FileUrls don't require a host, but AnyUrls do.
Viicos
reviewed
Oct 23, 2024
adriangb
approved these changes
Oct 23, 2024
|
@sydney-runkle Could it be that this PR made AnyHttpUrl unhashable? See #10955. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #9523
Fixes #7353
Fixes #10684
Sort of satisfying, I think #7353 is the first issue I opened (by directly copying / pasting a slack message, haha).
Depends on pydantic/pydantic-core#1488
Still need to add:
You can still annotate things with
UrlConstraints, but I'd recommend subclassing, especially ifhost_required=Trueand you want to get proper typing onhost(it used to always bestr | Noneas inherited fromUrlinpydantic-core, but now we control it on a class by class basis according to the correspondinghost_requiredconstraint).