Skip to content

Conversation

mgaligniana
Copy link
Contributor

Fixes GH-3516

@mgaligniana mgaligniana force-pushed the GH-3516-werkzeug-vendor-update branch 2 times, most recently from 75b9810 to dab33fb Compare September 12, 2025 16:09
Copy link

codecov bot commented Sep 12, 2025

Codecov Report

❌ Patch coverage is 44.44444% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.41%. Comparing base (5a122b5) to head (ba2c350).

Files with missing lines Patch % Lines
sentry_sdk/_werkzeug.py 44.44% 11 Missing and 4 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@ ## master #4793 +/- ## ========================================== - Coverage 84.47% 83.41% -1.07%  ========================================== Files 158 158 Lines 16506 16512 +6 Branches 2865 2864 -1 ========================================== - Hits 13944 13773 -171  - Misses 1712 1889 +177  Partials 850 850 
Files with missing lines Coverage Δ
sentry_sdk/_werkzeug.py 54.54% <44.44%> (+6.39%) ⬆️

... and 8 files with indirect coverage changes

@mgaligniana mgaligniana force-pushed the GH-3516-werkzeug-vendor-update branch 6 times, most recently from ba2c350 to 6238c21 Compare September 12, 2025 18:58
@mgaligniana mgaligniana marked this pull request as ready for review September 12, 2025 19:14
@mgaligniana mgaligniana requested a review from a team as a code owner September 12, 2025 19:14
cursor[bot]

This comment was marked as outdated.

@mgaligniana mgaligniana marked this pull request as draft September 15, 2025 12:54
@sentrivana
Copy link
Contributor

Thanks for the PR @mgaligniana -- I think the HTTP_X_FORWARDED_HOST handling needs to be added manually on top of the vendored in code. As far as I can tell we also did that in the current version as the original upstream 1.0.1 version also doesn't contain it.

@mgaligniana mgaligniana force-pushed the GH-3516-werkzeug-vendor-update branch from 6238c21 to b4ee7ae Compare September 17, 2025 03:10
@mgaligniana mgaligniana marked this pull request as ready for review September 17, 2025 03:17
@mgaligniana
Copy link
Contributor Author

mgaligniana commented Sep 17, 2025

Ok, Seer 🎉 liked!

But codecov not, should I add new tests for the new vendor?

Edited: I think yes because Anton said:

Also make sure this code is tested to have a high test coverage. (maybe by vendoring in the related tests from Werkzeug?)

But do you think I should copy-paste the test from werkzeug to get_host for example?

@sentrivana
Copy link
Contributor

But do you think I should copy-paste the test from werkzeug to get_host for example?

Tests would be great. I think the one you linked would be good, plus something custom for the HTTP_X_FORWARDED_HOST stuff.

@mgaligniana mgaligniana force-pushed the GH-3516-werkzeug-vendor-update branch 2 times, most recently from 2b7f7f1 to 71d1151 Compare September 19, 2025 03:23
@mgaligniana
Copy link
Contributor Author

So adding that now we have
image

@mgaligniana
Copy link
Contributor Author

There we go!

image
@mgaligniana mgaligniana force-pushed the GH-3516-werkzeug-vendor-update branch from 145a66a to 327eefe Compare September 25, 2025 16:05
cursor[bot]

This comment was marked as outdated.

@mgaligniana mgaligniana force-pushed the GH-3516-werkzeug-vendor-update branch from 327eefe to 3da87d1 Compare October 1, 2025 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants