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

Updates to Url types for Pydantic V2 #6638

Merged
merged 1 commit into from Jul 13, 2023
Merged

Updates to Url types for Pydantic V2 #6638

merged 1 commit into from Jul 13, 2023

Conversation

tpdorsey
Copy link
Contributor

@tpdorsey tpdorsey commented Jul 12, 2023

Review and update Url types, removing outdated information and updating examples.

Change Summary

Related issue number

Closes PYD-171

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 and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @davidhewitt

@linear
Copy link

linear bot commented Jul 12, 2023

PYD-171 TODO - Docs: URL Properties example

urls.md

URL Properties

try:

 MyDatabaseModel(db='postgres://user:passlocalhost:5432')

except ValidationError:

pass

# TODO the error output here is wrong!

# print(e)

@tpdorsey
Copy link
Contributor Author

please review

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.

👍 lgtm, just some offhand thoughts which you can freely dismiss 😁


The above types (which all inherit from `AnyUrl`) will attempt to give descriptive errors when invalid URLs are
provided:
- [`AnyUrl`][pydantic.networks.AnyUrl]: any scheme allowed, top-level domain (TLD) not required, host required.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it makes sense to tabulate these and maybe even generate the table programmatically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a good callout. I'll look into it today.

- `postgresql+psycopg2cffi`
- `postgresql+py-postgresql`
- `postgresql+pygresql`
- [`MySQLDsn`][pydantic.networks.MySQLDsn]: scheme `mysql`, user info required, TLD not required, host required.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we want to link to definitions of all of these, where possible? I assume it's easy enough for users to search for these though having links to e.g. mysql docs may be a nice touch? I suppose it's also a lot of effort to collect and maintain external links, so maybe not worthwhile.

@tpdorsey tpdorsey merged commit 78dde77 into main Jul 13, 2023
48 checks passed
@tpdorsey tpdorsey deleted the url-fixups branch July 13, 2023 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants