Skip to content

Included create_ssl_context function to create the same context with SSLConfig and serve as API - #996

Merged
lovelydinosaur merged 7 commits into
encode:masterfrom
cansarigol:create_ssl_context
Jul 27, 2020
Merged

Included create_ssl_context function to create the same context with SSLConfig and serve as API#996
lovelydinosaur merged 7 commits into
encode:masterfrom
cansarigol:create_ssl_context

Conversation

@cansarigol

@cansarigolcansarigol commented May 25, 2020

Copy link
Copy Markdown
Contributor

Refs #983

lovelydinosaur
lovelydinosaur reviewed May 25, 2020
View reviewed changes
Comment threaddocs/advanced.md
lovelydinosaur
lovelydinosaur reviewed May 25, 2020
View reviewed changes
Comment threaddocs/advanced.md Outdated
lovelydinosaur
lovelydinosaur reviewed May 25, 2020
View reviewed changes
Comment threadtests/test_config.py Outdated
@@ -38,10 +36,11 @@ def test_load_ssl_config_verify_env_file(https_server, ca_cert_pem_file, config)
else str(Path(ca_cert_pem_file).parent)
)
ssl_config = SSLConfig(trust_env=True)

@lovelydinosaurlovelydinosaurMay 25, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be great as part of this change if we dropped using the private SSLConfig from the tests completely.
Not necessarily a blocker to getting this PR in, but might be worth a go?

lovelydinosaur
lovelydinosaur reviewed May 25, 2020
View reviewed changes
Comment threadtests/test_config.py Outdated
lovelydinosaur
lovelydinosaur reviewed May 25, 2020
View reviewed changes

@lovelydinosaurlovelydinosaur left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic stuff! I've put a few inline comments here.

There's also a bit of thinking we'll want to do wrt. if http2 properly lives on the SSL configuration, or on the transport, or both, but I'm not going to go into that here.

@lovelydinosaurlovelydinosaur mentioned this pull request May 25, 2020
Closed
@lovelydinosaurlovelydinosaur mentioned this pull request Jul 24, 2020
Merged
@lovelydinosaurlovelydinosaur added this to the v0.14 milestone Jul 27, 2020
@lovelydinosaurlovelydinosaur added the enhancement New feature or request label Jul 27, 2020
lovelydinosaur
lovelydinosaur approved these changes Jul 27, 2020
View reviewed changes

@lovelydinosaurlovelydinosaur left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great yup!
I'll leave it open briefly in case anyone else also want to take a review onto it, but otherwise we'll get this into 0.14.
Thanks so much !

florimondmanca
florimondmanca approved these changes Jul 27, 2020
View reviewed changes

@florimondmancaflorimondmanca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@lovelydinosaur
lovelydinosaur merged commit 926a55a into encode:masterJul 27, 2020
@KludexKludex mentioned this pull request May 11, 2026
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lovelydinosaurlovelydinosaurlovelydinosaur approved these changes
+1 more reviewer
@florimondmancaflorimondmancaflorimondmanca approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancementNew feature or request

Milestone

v0.14

Development

Successfully merging this pull request may close these issues.

3 participants

@cansarigol@lovelydinosaur@florimondmanca