pyOpenSSL: Adapt to changes in 22.0.0 #7080
Conversation
These methods only accept str The docstrings were fixed for some of these methods in pyca/pyopenssl@41ceefb
lovetox
force-pushed
the
master
branch
2 times, most recently
from
9f3a029 to
784d613
Compare
- pyOpenSSL dropped python2 compatibility with 22.0.0 - Replace Text alias with str - Don’t inherit from object
stubs/pyOpenSSL/OpenSSL/crypto.pyi
Outdated
| @@ -129,7 +129,7 @@ class X509Store: | |||
| def __init__(self) -> None: ... | |||
| def add_cert(self, cert: X509) -> None: ... | |||
| def add_crl(self, crl: CRL) -> None: ... | |||
| def load_locations(self, cafile: Text | bytes, capath: Text | bytes | None = ...) -> None: ... | |||
| def load_locations(self, cafile: str | bytes, capath: str | bytes | None = ...) -> None: ... | |||
There was a problem hiding this comment.
cafile and capath should be annotated with _typeshed.StrOrBytesPath. They get passed to _path_bytes which calls os.fspath.
There was a problem hiding this comment.
I added the suggestions
| def load_crl(type: int, buffer: str | bytes) -> CRL: ... | ||
| def load_pkcs7_data(type: int, buffer: str | bytes) -> PKCS7: ... | ||
| def load_pkcs12(buffer: str | bytes, passphrase: bytes | None = ...) -> PKCS12: ... | ||
| def sign(pkey: PKey, data: str | bytes, digest: str) -> bytes: ... |
There was a problem hiding this comment.
Idea for future PR: Some arguments, such as data here, get passed to text_to_bytes_and_warn, which emits a DeprecationWarning if you pass a string. We should probably just change them to bytes in typeshed, so that the warning happens when type checking rather than at runtime.
There was a problem hiding this comment.
Im curious, is this a standard thing done in the typing community?
This seems to force application maintainers into one of the following options if they use a deprecated type
- They could add a
# type: ignorewhich stops typing of more stuff than we want - They could not update to the newest version of these stubs and miss out on changes which fixed wrong typings
- They could use the new type, which may or may not be possible depending on the project
Don’t seem very good options to me
Especially because DeprecationWarnings can be catched by unittests.
But i don’t have much experience with typing so maybe this is standard procedure in the community and its not a big deal.
There was a problem hiding this comment.
We currently don't have a good way to mark deprecated things. There's some discussion about this here: python/typing#1043
For now, we usually keep deprecated things in the spirit of avoiding false positives. This was just a random idea.
|
After a review, please don't |
Fix digest argument type, these methods only accept str
The docstrings were fixed for some of these methods in pyca/pyopenssl@41ceefb
pyOpenSSL dropped python2 compatibility with 22.0.0
Replace Text alias with str
Don’t inherit from object