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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check if we still need _werkzeug.py
2 participants