Skip to content

Moved create and verify ssl context function into the utils and included lru cache. - #986

Closed
cansarigol wants to merge 1 commit into
encode:masterfrom
cansarigol:cache_ssl_context
Closed

Moved create and verify ssl context function into the utils and included lru cache.#986
cansarigol wants to merge 1 commit into
encode:masterfrom
cansarigol:cache_ssl_context

Conversation

@cansarigol

@cansarigolcansarigol commented May 23, 2020

Copy link
Copy Markdown
Contributor

Refs #978

globally caching our SSL contexts suggestion of @tomchristie .

@cansarigol
cansarigolforce-pushed the cache_ssl_context branch 5 times, most recently from 4bab0af to cb78e5eCompareMay 24, 2020 12:40
@florimondmanca

florimondmanca commented May 24, 2020

Copy link
Copy Markdown
Contributor

Hi @cansarigol, thanks for tackling this!

From what I can see, this PR tries to address both #978 and #983. It's probably best that we keep fixes for these separate.

Most likely #983 is higher priority, as it would make SSLConfig go away (and this PR currently uses it for the other part).

@cansarigol

cansarigol commented May 25, 2020

Copy link
Copy Markdown
ContributorAuthor

Hi @florimondmanca , I thought that we can handle in a single PR and I can include a test about SSLConfig().ssl_context == httpx.create_ssl_context() . Separately better as you said.

@lovelydinosaur

Copy link
Copy Markdown
Contributor

Closing in favour of #996

@lovelydinosaur

lovelydinosaur commented May 25, 2020

Copy link
Copy Markdown
Contributor

Incidentally let's not jump straight in with implementing SSLContext caching once #996 is in.
I think that aspect is worth considering, but I don't think it's necessarily a given.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@cansarigol@florimondmanca@lovelydinosaur