Skip to content

Conversation

sydney-runkle
Copy link
Contributor

@sydney-runkle sydney-runkle commented Oct 9, 2024

Alternative to #10500

Fixes #9251

Had to decouple (though, I view this as a sort of simplification) the EncodedBytes and EncodedStr dataclases. The API here feels a bit confusing- having the encoder separate from these types that define the pydantic methods seems overly complex.

Additionally, the EncodedStr type (unless I'm misunderstanding) does quite a large amount of encoding and decoding for both validation and serialization...

The main fix here is using handler(source) as the schema for the validator functions so that any constraints applied to the original type are respected.

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

cloudflare-workers-and-pages bot commented Oct 9, 2024

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8b4f35d
Status: ✅  Deploy successful!
Preview URL: https://e998034b.pydantic-docs.pages.dev
Branch Preview URL: https://bytes-fix.pydantic-docs.pages.dev

View logs

Copy link

codspeed-hq bot commented Oct 9, 2024

CodSpeed Performance Report

Merging #10584 will not alter performance

Comparing bytes-fix (8b4f35d) with main (6c3d3b3)

Summary

✅ 38 untouched benchmarks

@@ -2580,7 +2593,7 @@ def decode_str(self, data: bytes, _: core_schema.ValidationInfo) -> str:
Returns:
The decoded data.
"""
return data.decode()
return self.encoder.decode(data.encode()).decode()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So that I understand the intent here - we validate a str, then encode -> bytes, decode with our encoder, then decode back to a str? 🤮

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, since the encoder operates on bytes I think you need to do this if you want to apply it to strings

Copy link
Contributor

github-actions bot commented Oct 9, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  pydantic
  types.py
Project Total  

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

@sydney-runkle sydney-runkle requested a review from Viicos October 10, 2024 14:30
@sydney-runkle
Copy link
Contributor Author

I think this is the right behavior, but if we have any issues with this type in v2.10, this would be the PR to revisit. Seems pretty low stakes, though...

@sydney-runkle sydney-runkle merged commit 76222d2 into main Oct 11, 2024
61 checks passed
@sydney-runkle sydney-runkle deleted the bytes-fix branch October 11, 2024 14:29
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.

Base64Bytes field doesn't raises validation error for min_length constraint
2 participants