Skip to content

Subclass all single host url classes from AnyUrl to preserve behavior from v2.9#10856

Merged
sydney-runkle merged 2 commits intomainfrom
any-url-base-type
Nov 18, 2024
Merged

Subclass all single host url classes from AnyUrl to preserve behavior from v2.9#10856
sydney-runkle merged 2 commits intomainfrom
any-url-base-type

Conversation

@sydney-runkle
Copy link
Copy Markdown
Contributor

Addresses concerns from #10783 (comment)

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Nov 15, 2024
@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 48d1503
Status: ✅  Deploy successful!
Preview URL: https://4662b687.pydantic-docs.pages.dev
Branch Preview URL: https://any-url-base-type.pydantic-docs.pages.dev

View logs

Comment thread pydantic/networks.py
def host(self) -> str:
"""The required URL host."""
return self._url.host # type: ignore
return self._url.host # pyright: ignore[reportReturnType]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

cc @Viicos, using more explicit ignores :)

Comment thread pydantic/networks.py
Comment on lines -490 to -494
@property
def host(self) -> str:
"""The required URL host."""
return self._url.host # type: ignore

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can remove the added host specs that overrode the str | None return type from _BaseUrl, as AnyUrl enforces str

Comment thread pydantic/networks.py
Comment on lines +610 to +614
@property
def host(self) -> str | None: # pyright: ignore[reportIncompatibleMethodOverride]
"""The host part of the URL, or `None`."""
return self._url.host

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Have to add this for the case where host_required=Fals because AnyUrl has host typed as str, so we indicate the optional nature here.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Nov 15, 2024

CodSpeed Performance Report

Merging #10856 will not alter performance

Comparing any-url-base-type (48d1503) with main (11d04f3)

Summary

✅ 44 untouched benchmarks

@github-actions
Copy link
Copy Markdown
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  pydantic
  networks.py 613, 628, 709
Project Total  

This report was generated by python-coverage-comment-action

Copy link
Copy Markdown
Collaborator

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Based on the surrounding discussion, this makes sense to me 👍

@sydney-runkle sydney-runkle merged commit 372d4e2 into main Nov 18, 2024
@sydney-runkle sydney-runkle deleted the any-url-base-type branch November 18, 2024 13:08
sydney-runkle added a commit that referenced this pull request Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

relnotes-fix Used for bugfixes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants