8000 use pydantic's version in error messages by davidhewitt · Pull Request #759 · pydantic/pydantic-core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

use pydantic's version in error 8000 messages #759

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 10, 2023
Merged

use pydantic's version in error messages #759

merged 1 commit into from
Jul 10, 2023

Conversation

davidhewitt
Copy link
Contributor
@davidhewitt davidhewitt commented Jul 10, 2023 •
edited by pydantic-hooky bot
Loading

Change Summary

As pydantic and pydantic-core versions have decoupled, when rendering errors we should use the pydantic package error messages, if available.

Related issue number

N/A

Checklist

  • Unit tests for the changes exist
  • Documentation reflects the changes where applicable
  • Pydantic tests pass with this pydantic-core (except for expected changes)
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @adriangb

@codspeed-hq
Copy link
codspeed-hq bot commented Jul 10, 2023

CodSpeed Performance Report

Merging #759 will not alter performances

Comparing dh/error-versions (8355b71) with main (dfbedbd)

Summary

✅ 126 untouched benchmarks

@davidhewitt
Copy link
Contributor Author

please review

@davidhewitt davidhewitt merged commit 45d812f into main Jul 10, 2023
@davidhewitt davidhewitt deleted the dh/error-versions branch July 10, 2023 19:20
adriangb pushed a commit that referenced this pull request Jul 11, 2023
@davidhewitt @adriangb
use pydantic's version in error messages (#759)
e7f9ecf
@adriangb
Copy link
Member

Just a thought after we merged this: wouldn't it be simpler and make more sense to require an error_url_format_template: str or something like that in the validate_* methods so that pydantic can change the error URL without impacting pydantic-core (including setting the version)?

@adriangb
Copy link
Member

Because now this PR means that pydantic-core has "some knowledge" of pydantic and pedantic obviously has knowledge of pydantic-core, so it's very much a circular dependency. Not to mention pydantic-core is totally unusable without pydantic, which make the whole point of having a separate package seem totally absurd.

@davidhewitt
Copy link
Contributor Author

Just a thought after we merged this: wouldn't it be simpler and make more sense to require an error_url_format_template: str or something like that in the validate_* methods so that pydantic can change the error URL without impacting pydantic-core (including setting the version)?

Sure, can you give me a little bit more detail on how this might all hang together? I could take a look at moving to that pattern if it seems reasonable.

@adriangb
Copy link
Member

I was thinking add an argument to SchemaValidator.validate_python and validate_json that has some sort of string format template (ideally one that is meaningful in both Python and rust). Then pydantic would do SchemaValidator(...).validate_python(..., error_template_url=ERROR_TEMPLATE_URL) where ERROR_TEMPLATE_URL is some global along the lines of ERR 7BFB OR_TEMPLATE_URL=f'https://pydantic.dev/{__version__}/errors/{error_type}/). Not sure if that would fully work or make sense, but it does seem like it'd be simpler and result in less circular dependencies between the pakcages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adriangb adriangb adriangb approved these changes

Assignees

@adriangb adriangb

Labels
ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@davidhewitt @adriangb
0