Skip to content
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

make order of headers repeatable #474

Merged
merged 2 commits into from Jan 1, 2022

Conversation

grubberr
Copy link
Contributor

Signed-off-by: Sergey Chvalyuk grubberr@gmail.com

Closes #

Checklist

  • Added docstrings and type annotations
  • Added test coverage
  • Updated changelog to describe any user-facing features or changed behavior

Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
@codecov-commenter
Copy link

Codecov Report

Merging #474 (3d5f55d) into master (71ea521) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #474   +/-   ##
=======================================
  Coverage   98.75%   98.76%           
=======================================
  Files          18       18           
  Lines        1290     1291    +1     
  Branches      185      186    +1     
=======================================
+ Hits         1274     1275    +1     
  Misses          8        8           
  Partials        8        8           
Impacted Files Coverage Δ
requests_cache/cache_keys.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 71ea521...3d5f55d. Read the comment docs.

@JWCook JWCook added this to the v0.9 milestone Dec 17, 2021
@JWCook JWCook added the bug label Dec 17, 2021
@JWCook
Copy link
Member

JWCook commented Dec 17, 2021

Looks like this fixes a regression bug; previously headers were correctly sorted (via normalize_request()), but I see that converting those to a set in get_matched_headers() means the order wasn't guaranteed anymore. Thanks for fixing that!

Looks like there's just one minor complaint from black, and a type warning from mypy. And could you also add a unit test that covers this?

@JWCook JWCook force-pushed the improve_get_matched_headers branch from 28f368a to 99b7793 Compare January 1, 2022 17:59
@JWCook JWCook merged commit 3470482 into requests-cache:master Jan 1, 2022
@JWCook
Copy link
Member

JWCook commented Jan 3, 2022

Updated and merged. This is included in the latest release (0.9). Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants