Add CA.from_pem #107
Add CA.from_pem #107
Conversation
Codecov Report
@@ Coverage Diff @@
## master #107 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 3 3
Lines 359 371 +12
Branches 23 23
=====================================
+ Hits 359 371 +12
|
|
|
||
| Returns: | ||
| CA: the newly-generated certificate authority | ||
| """ |
There was a problem hiding this comment.
Does this show up in the docs automatically, or do we need to add it?
There was a problem hiding this comment.
Do you mean the Returns part? It shows up like this in the docs:
Without it, this part disappears
There was a problem hiding this comment.
Oh no, I meant the whole docstring :-). That answers the question though!
trustme/__init__.py
Outdated
| .format(ctx.__class__.__name__)) | ||
|
|
||
| @classmethod | ||
| def from_pem(cls, cert_path, private_key_path): |
There was a problem hiding this comment.
Taking paths limits this to certs that are stored in files on disk, which is kind of an arbitrary limitation. Maybe it should accept the actual PEM data instead? (Or file objects, but that feels unnecessarily complicated when we're talking about data that's a few kilobytes at most.)
There was a problem hiding this comment.
Sure. This is up to you! I can accept trustme.Blobs, or bytes, or file-like objects
There was a problem hiding this comment.
(My reasoning was that if you use from_pem then you're likely to have the files on disk, but well, this kind of thinking led to only accept certificates on disks in many APIs, so it's flawed indeed)
There was a problem hiding this comment.
I've implemented the bytes option. It certainly simplifies the code! Please take another look.
|
Thank you for the quick review! 💯 |
|
Thanks! Should I give you pypi rights so you can roll a release? What's your pypi name? |
|
Thank you! Yes, I'd like to roll a release. Here's my PyPI account: https://pypi.org/user/quentinp/ |
|
ok, you should be good to go
…On Wed, Oct 30, 2019 at 11:54 PM Quentin Pradet ***@***.***> wrote:
Thank you! Yes, I'd like to roll a release. Here's my PyPI account:
https://pypi.org/user/quentinp/
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#107?email_source=notifications&email_token=AAEU42HWU6RMOBQ5CJHUT4TQRJ6LNA5CNFSM4JGZV6G2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECWXQ2Y#issuecomment-548239467>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEU42ET75JBAGPM6LAMHULQRJ6LNANCNFSM4JGZV6GQ>
.
--
Nathaniel J. Smith -- https://vorpus.org <http://vorpus.org>
|
|
Thank you! 0.5.3 is out with this change. |
2 participants
Part of my work on urllib3/urllib3#1705. The goal is to start using trustme more in urllib3, but by doing it gradually, and only switch to a trustme-generated CA at the end of the process.