-
Notifications
You must be signed in to change notification settings - Fork 236
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
implement omit_trailing_slash feature for PyUrl [7186] #1218
base: main
Are you sure you want to change the base?
Conversation
please review |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1218 +/- ##
==========================================
+ Coverage 90.21% 90.29% +0.08%
==========================================
Files 106 106
Lines 16339 16423 +84
Branches 36 39 +3
==========================================
+ Hits 14740 14829 +89
+ Misses 1592 1575 -17
- Partials 7 19 +12
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
13965b7
to
ca4b802
Compare
CodSpeed Performance ReportMerging #1218 will not alter performanceComparing Summary
|
Prior art: #1173. Is there a reason why this route should be seen as preferred to the broader suite of normalization bugs which that PR discussed (and ultimately decided we need fixes in |
In my humble opinion, there seem to be two related yet slightly different issues that we are trying to tackle. The #1173 PR is aimed at fixing the broken Url.build feature, as discussed earlier, and this should be addressed through modifications to rust-url. Modifying the behavior of rust-url to accommodate URL processing, a functionality relied upon by all users of this crate, appears to be quite challenging. However, from the perspective of pydantic, this was the previous behavior that many projects accepted and utilized, and they aim to continue using it in v2. What are your thoughts on this? |
Let me reflect on this. My instinct is that this is still a |
Thanks! |
(Just to set expectations, I've set myself a reminder to explore this next week, when I expect to be also picking up #1085 again. Please ping me if this PR falls off my radar.) |
Great! I'll glad to any response) |
@davidhewitt Hey, could you please provide updates?) |
I have been thinking about this a bit. I would like to avoid the config flag if we can. It seems to me that it's a problem that the URL does not round-trip, and this seems to be a problem of |
@davidhewitt, what do next steps look like here? Do we still plan on attempting to implement this? |
Change Summary
Based on comment pydantic/pydantic#7186 (comment) implemented
omit_trailing_slash
feature for PyUrl crate.Related issue number
pydantic/pydantic#7186
Checklist
pydantic-core
(except for expected changes)Selected Reviewer: @dmontagu