New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prevent duplicate default SSLContext instances #105348
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Hey there @jbouwh, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
CLA signed! 🖊️ |
This looks like a good fix. Please split it into two PRs so it can be backported One for imap |
a98e73f
to
ea24063
Compare
Okay. I split the IMAP change off into #105362 and left only the util changes in this PR. I guess that one will need to go first, then? In the meantime, this one is going to fail |
Thanks @vexofp |
Co-authored-by: J. Nick Koston <nick@koston.org>
needs #105362
Proposed change
This PR fixes the unintentional duplication of default
SSLContext
instances created inutil/ssl.py
(#105115), when the factory methods are called both with and without explicitly passing the default cipher list. The root cause of this behavior is that the@functools.cache
decorator does not play nicely with default arguments. While the performance/efficiency impact of these duplicate contexts is quite small, so is the fix. 😄The approach taken was to move the implementation and caching into private functions that do not have a default argument value. There are no changes to the actual implementation. The existing public functions now call those private functions, always passing in the argument explicitly to ensure reliable cache behavior. The public functions maintain their existing signatures (including the defaulted argument value), so there is no change to the public API.
A new test was added to verify the caching behavior.
The change in
imap/coordinator.py
is to resolve amypy
warning that cropped up. I suspect the@cache
decorator was obscuring some of the type information previously. Now that it is removed from the public functions,mypy
correctly caught that astr
was incorrectly being passed where an enumeration value is required.Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: