Skip to content

Conversation

sydney-runkle
Copy link
Contributor

Addresses concerns from #10783 (comment)

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

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

@@ -475,10 +475,14 @@ class AnyUrl(_BaseUrl):
@property
def host(self) -> str:
"""The required URL host."""
return self._url.host # type: ignore
return self._url.host # pyright: ignore[reportReturnType]
Copy link
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 on lines -490 to -494
@property
def host(self) -> str:
"""The required URL host."""
return self._url.host # type: ignore

Copy link
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 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
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.

Copy link

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

Copy link
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
Contributor

@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
54 checks passed
@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