Skip to content

Conversation

@Akuli
Copy link
Collaborator

@Akuli Akuli commented May 23, 2022

Fixes a problem reported by @rpdelaney in #7773 (comment)

I spent a while looking at requests source code, and I'm quite convinced that the headers can't be mutated. Each request is prepared before auth etc headers are applied, and the headers are copied when preparing.

@JelleZijlstra
Copy link
Member

Thanks! Is it worth adding some test cases here? We've gone through quite a few iterations with these requests mappings.

@Akuli
Copy link
Collaborator Author

Akuli commented May 23, 2022

Most of the iterations were before mypy_primer worked for third-party stubs. Ideally we would just add @rpdelaney's project to mypy_primer, if it happens to be open-source.

@github-actions

This comment has been minimized.

@Akuli
Copy link
Collaborator Author

Akuli commented May 23, 2022

Actually it is open-source.

@Akuli Akuli closed this May 23, 2022
@Akuli Akuli reopened this May 23, 2022
@Akuli
Copy link
Collaborator Author

Akuli commented May 23, 2022

closed accidentally

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood
Copy link
Member

AlexWaygood commented May 24, 2022

Most of the iterations were before mypy_primer worked for third-party stubs. Ideally we would just add @rpdelaney's project to mypy_primer, if it happens to be open-source.

+1 to adding more projects to mypy-primer if they use code that isn't otherwise covered. But I feel like "adding more projects to mypy-primer" and "adding specific regression tests" aren't necessarily mutually exclusive.

The huge value of mypy-primer is that it can detect problems that we don't even know are problems yet. But the disadvantage is we'd instantly lose coverage if a certain project decided to move from using requests to HTTPie (or whatever). We have more control over the regression tests in the test_cases directory.

@rpdelaney
Copy link

downforeveryone is Apache 2.0 licensed. I don't know how mypy_primer works, but can't you check out a package at a particular commit? Or just copy-paste in the code you need to check the regression.

@AlexWaygood
Copy link
Member

downforeveryone is Apache 2.0 licensed. I don't know how mypy_primer works, but can't you check out a package at a particular commit? Or just copy-paste in the code you need to check the regression.

I think we probably all agree that adding downforeverone to mypy-primer would be both Possible And Good — @Akuli's filed hauntsaninja/mypy_primer#33 to track that :)

What we're debating is whether to also add regression tests to typeshed itself via our fairly new test_cases directory. We currently have a smattering of test cases for the stdlib stubs, but no test cases as yet for the third-party stubs typeshed maintains.

@JelleZijlstra JelleZijlstra merged commit f77d0f8 into python:master May 26, 2022
@Akuli Akuli deleted the headers branch May 27, 2022 09:24
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.

4 participants