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

Fix cryptography WithSerialization aliases #8239

Closed
wants to merge 1 commit into from
Closed

Fix cryptography WithSerialization aliases #8239

wants to merge 1 commit into from

Conversation

hasier
Copy link
Contributor

@hasier hasier commented Jul 5, 2022

Align with changes in pyca/cryptography@6a8c0b5, still present in latest commit in main.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2022

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

@hasier
Copy link
Contributor Author

hasier commented Jul 5, 2022

@AlexWaygood I see you have been one of the latest contributors to cryptography, could you help me understand why the 3rd party stubtest is failing? I'm not too sure I understand why it's not finding private_bytes and private_numbers, they are indeed part of the interfaces in the latest commit. Does it have to do with the stubtest_allowlist.txt (I'm not sure how this interacts in the tests either tbh 😅 )?

@AlexWaygood
Copy link
Member

AlexWaygood commented Jul 5, 2022

Hi, @hasier -- thanks for the PR!

I think stubtest is probably failing because the version field in the METADATA.toml file for these stubs is pinned to 3.3:

. This is because cryptography has shipped with inline type annotations for over a year now, so the stubs typeshed ships are now obsolete -- you should use the type hints provided by the upstream library instead.

Usually in a situation like this, we would have deleted the stubs from typeshed by now. However, in this case things are a little more complicated due to the fact that other stubs packages in typeshed depend on these stubs (#5618).

If you'd like to help out with typeshed, a really great contribution to make would be to put some work into tweaking our testing infrastructure so that we can include non-stub packages as dependencies for typeshed stubs. Once we're able to do that, we'll be able to remove these out-of-date cryptography stubs (#5768).

For now, I think it's probably not worth putting the effort into improving these long-obsolete stubs, sadly :(

@hasier
Copy link
Contributor Author

hasier commented Jul 5, 2022

Ah I see. I had not realised they already had inline annotations because we are not in the most recent version of cryptography, so some of them were missing 🤦 I'll close this PR then, thanks for explaining @AlexWaygood!

@hasier hasier closed this Jul 5, 2022
@hasier hasier deleted the cryptography-with-serialization branch July 5, 2022 15:13
@AlexWaygood
Copy link
Member

Ah I see. I had not realised they already had inline annotations because we are not in the most recent version of cryptography, so some of them were missing 🤦 I'll close this PR then, thanks for explaining @AlexWaygood!

No problem — and thanks again for the PR!

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.

2 participants