Add more stubs for cryptography #3307

jlaine · 2019-10-05T00:10:58Z

No description provided.

jlaine · 2019-10-05T00:22:31Z

The x509 stub still needs some love, I haven't had a chance to properly represent the "Name" class.

JelleZijlstra · 2019-10-05T02:56:45Z

third_party/2and3/cryptography/hazmat/backends/interfaces.pyi

@@ -0,0 +1,13 @@
+class CMACBackend: ...


Presumably these things have some methods? If you don't have time to stub them all out, we should add a def __getattr__(self, name: str) -> Any to avoid false positives for people using these classes.

The backends themselves are private API.. so it's debatable whether they should be declared at all.

They are documented, though? Also, they should all have a metaclass=ABCMeta base. Same is true for several of the interfaces in other files.

jlaine · 2019-10-05T14:31:44Z

One thing I noticed is that the signature for load_pem_private_key looks wrong, I believe the password argument is bytes, not str.

jlaine · 2019-10-05T21:10:28Z

CI seems to be failing because Python 2 lacks ipaddress, see #3309.

It's a bit annoying having to deal with Python 2 less than 3 months before EOL..

Edit: this is no longer an issue, ipaddress is now in third_party/2

JelleZijlstra · 2019-10-07T03:07:02Z

I think the EOL of Python 2 in typeshed is probably going to be later than December 31.

srittau

Thank you for these stubs! I left some remarks below, not a full review yet.

third_party/2and3/cryptography/exceptions.pyi

srittau · 2019-10-07T12:52:07Z

third_party/2and3/cryptography/hazmat/backends/interfaces.pyi

@@ -0,0 +1,13 @@
+class CMACBackend: ...


They are documented, though? Also, they should all have a metaclass=ABCMeta base. Same is true for several of the interfaces in other files.

third_party/2and3/cryptography/hazmat/primitives/asymmetric/dsa.pyi

jlaine · 2019-10-18T19:53:11Z

@srittau I have added a lot more stubs, feel free to review.

I have extensively read the cryptography documentation, and AFAICT the only module which is not yet fully typed is x509.

srittau

This is very helpful, thank you for the stubs! I am sorry for the delay in reviewing, other things came up. For now I just managed to review up to hazmat/primitives/asymmetric, but I hope to review the rest soon.

A few notes below, none of them showstoppers. Most of them are just repeats of the same issue, but I pointed them out in each place to (hopefully) make it easier to fix.

third_party/2and3/cryptography/fernet.pyi

third_party/2and3/cryptography/exceptions.pyi

third_party/2and3/cryptography/fernet.pyi

third_party/2and3/cryptography/hazmat/primitives/asymmetric/padding.pyi

third_party/2and3/cryptography/hazmat/primitives/asymmetric/rsa.pyi

third_party/2and3/cryptography/hazmat/primitives/asymmetric/utils.pyi

srittau

This is very helpful, thank you for the stubs! I am sorry for the delay in reviewing, other things came up. For now I just managed to review up to hazmat/primitives/asymmetric, but I hope to review the rest soon.

A few notes below, none of them showstoppers. Most of them are just repeats of the same issue, but I pointed them out in each place to (hopefully) make it easier to fix.

jlaine · 2019-10-30T13:58:01Z

Thanks so much for taking the time for this first review, I've addressed all the issues you pointed out. The changes related to abstract properties seem to have broken CI though, it looks as though I need to go over all the subclasses to add the corresponding concrete properties.

alex · 2019-10-30T14:02:56Z

third_party/2and3/cryptography/hazmat/backends/interfaces.pyi

+)
+
+class CipherBackend(metaclass=ABCMeta):
+    @abstractmethod


I would not bother including the various backend API methods -- basically no user should ever be invoking them, so it's a lot of code for relatively little benefit (that's my opinion at least, maybe the approach of typeshed is different)

That was my opinion too, but as @srittau pointed out in #3307 (comment) these methods are part of the cryptography documentation.. so public API?

jlaine · 2019-10-30T18:02:33Z

Since @alex is weighing in on this PR, it might be a good time to decide whether these type hints should live in cryptography itself or it typeshed.

I'm happy with both approaches, as long as it lets me typecheck my code :)

alex · 2019-10-30T21:35:57Z

We're unlikely to be convinced to bring these in-tree for cryptography until we're python3 only and can use the native syntax.

srittau · 2019-11-03T18:05:50Z

@alex I'd just like to point out that the syntax of stubs is independent of the supported Python version. Even when checking Python 2.7 code, type checkers will support stub files using newer features.

srittau

Finished the review now. Just one more note below. Please note that due to the size of the PR, I didn't compare the stubs against implementation or documentation like I usually do, I just looked for obvious issues.

third_party/2and3/cryptography/hazmat/primitives/ciphers/modes.pyi

alex · 2019-11-03T18:24:00Z

Yes, I understand the stubs syntax works in both Python2/3. We don't plan to adopt type hints until they can be done directly inline in the source code.

srittau · 2019-11-03T18:30:13Z

Seems sensible. We are happy to have them in typeshed as long as required.

srittau · 2019-11-04T09:59:19Z

Looks good, I will merge when the build failure is fixed.

jlaine · 2019-11-04T11:14:39Z

Let's see if CI completes this time. I caught a couple of other places in hazmat.ciphers.algorithms where abstract properties were needed instead of members.

Going forward, I don't plan to do a "hit and run", please mention me if any issues crop up related to these new stubs. I also plan to expand the x509 module as time allows.

jlaine · 2019-11-04T18:42:10Z

Thanks for all your guidance @srittau, this is one friendly project!

jlaine mentioned this pull request Oct 5, 2019

Split the cryptography (and other?) typestubs into independent repo? aiortc/aioquic#20

Closed

JelleZijlstra reviewed Oct 5, 2019

View reviewed changes

jlaine force-pushed the cryptography branch from e5228a2 to cd2262d Compare October 5, 2019 14:28

jlaine force-pushed the cryptography branch from cd2262d to aba1119 Compare October 5, 2019 14:33

srittau reviewed Oct 7, 2019

View reviewed changes

jlaine force-pushed the cryptography branch 12 times, most recently from 6645405 to dde4a57 Compare October 18, 2019 15:22

jlaine mentioned this pull request Oct 18, 2019

Adding type hinting for cryptography pyca/cryptography#4275

Closed

jlaine force-pushed the cryptography branch 3 times, most recently from 4fe102a to 353ba30 Compare October 18, 2019 19:20

jlaine mentioned this pull request Oct 18, 2019

Correctly document backend argument of KBKDFHMAC pyca/cryptography#5026

Merged

jlaine force-pushed the cryptography branch from 353ba30 to 6c9b2d9 Compare October 18, 2019 22:49

Add more stubs for cryptography

5fd41f9

jlaine force-pushed the cryptography branch from 6c9b2d9 to 5fd41f9 Compare October 18, 2019 23:22

srittau requested changes Oct 30, 2019

View reviewed changes

jlaine force-pushed the cryptography branch from 6d35e56 to c34a4c3 Compare October 30, 2019 13:51

jlaine force-pushed the cryptography branch from c34a4c3 to 68457a4 Compare October 30, 2019 17:15

Address first round of review comments

aa0cc02

jlaine force-pushed the cryptography branch from 68457a4 to aa0cc02 Compare October 30, 2019 17:43

Move misplaced MAC modules

ab7ce1a

alex reviewed Oct 30, 2019

View reviewed changes

srittau requested changes Nov 3, 2019

View reviewed changes

third_party/2and3/cryptography/hazmat/primitives/ciphers/modes.pyi Outdated Show resolved Hide resolved

srittau approved these changes Nov 4, 2019

View reviewed changes

Address second round of review comments

6aa1ded

jlaine force-pushed the cryptography branch from 44ad57a to 6aa1ded Compare November 4, 2019 11:04

srittau merged commit 047caa9 into python:master Nov 4, 2019

jlaine deleted the cryptography branch November 6, 2019 00:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add more stubs for cryptography #3307

Add more stubs for cryptography #3307

jlaine commented Oct 5, 2019

jlaine commented Oct 5, 2019

JelleZijlstra Oct 5, 2019

jlaine Oct 5, 2019

srittau Oct 7, 2019

jlaine commented Oct 5, 2019

jlaine commented Oct 5, 2019 •

edited

Loading

JelleZijlstra commented Oct 7, 2019

srittau left a comment

srittau Oct 7, 2019

jlaine commented Oct 18, 2019 •

edited

Loading

srittau left a comment

srittau left a comment

jlaine commented Oct 30, 2019

alex Oct 30, 2019

jlaine Oct 30, 2019

jlaine commented Oct 30, 2019 •

edited

Loading

alex commented Oct 30, 2019

srittau commented Nov 3, 2019

srittau left a comment

alex commented Nov 3, 2019

srittau commented Nov 3, 2019

srittau commented Nov 4, 2019

jlaine commented Nov 4, 2019 •

edited

Loading

jlaine commented Nov 4, 2019

Add more stubs for cryptography #3307

Add more stubs for cryptography #3307

Conversation

jlaine commented Oct 5, 2019

jlaine commented Oct 5, 2019

JelleZijlstra Oct 5, 2019

Choose a reason for hiding this comment

jlaine Oct 5, 2019

Choose a reason for hiding this comment

srittau Oct 7, 2019

Choose a reason for hiding this comment

jlaine commented Oct 5, 2019

jlaine commented Oct 5, 2019 • edited Loading

JelleZijlstra commented Oct 7, 2019

srittau left a comment

Choose a reason for hiding this comment

srittau Oct 7, 2019

Choose a reason for hiding this comment

jlaine commented Oct 18, 2019 • edited Loading

srittau left a comment

Choose a reason for hiding this comment

srittau left a comment

Choose a reason for hiding this comment

jlaine commented Oct 30, 2019

alex Oct 30, 2019

Choose a reason for hiding this comment

jlaine Oct 30, 2019

Choose a reason for hiding this comment

jlaine commented Oct 30, 2019 • edited Loading

alex commented Oct 30, 2019

srittau commented Nov 3, 2019

srittau left a comment

Choose a reason for hiding this comment

alex commented Nov 3, 2019

srittau commented Nov 3, 2019

srittau commented Nov 4, 2019

jlaine commented Nov 4, 2019 • edited Loading

jlaine commented Nov 4, 2019

jlaine commented Oct 5, 2019 •

edited

Loading

jlaine commented Oct 18, 2019 •

edited

Loading

jlaine commented Oct 30, 2019 •

edited

Loading

jlaine commented Nov 4, 2019 •

edited

Loading