Support warm-up of loader caches#1462
Conversation
|
Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services. |
726bb38 to
eca8210
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1462 +/- ##
==========================================
+ Coverage 91.60% 91.67% +0.07%
==========================================
Files 76 76
Lines 8078 8148 +70
==========================================
+ Hits 7400 7470 +70
Misses 678 678
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| ) | ||
| return service_data | ||
|
|
||
| def warm_up_loader_caches( |
There was a problem hiding this comment.
For advanced use cases, including non-default executors, this method may be called explicitly before client creation.
d2bc4da to
74d9cef
Compare
7eb8690 to
c60df2e
Compare
| def warm_up_loader_caches( | ||
| self, | ||
| service_name: str, | ||
| api_version: Optional[str] = None, |
There was a problem hiding this comment.
(nit) optional is considered confusing. Best use union of None and whatever's "optional".
There was a problem hiding this comment.
We still support Python 3.9 and cannot use the modern str | None syntax introduced in Python 3.10. Those deprecated Optional and Union annotations will be replaced once botocore and we drop support for Python 3.9.
|
I can't really review this in-depth, just left a few infra/testing comments. |
Fair enough and thank you anyway! |
53c2115 to
95b54a2
Compare
|
I tried to validate this in Home Assistant (IDrive e2 backup provider), but Home Assistant is currently pinned to aiobotocore 2.x. Would it be possible to make this warm-up API available (or backported) on the 2.x line as well, so downstream projects pinned to 2.x can adopt and test it? |
We typically don't provide backports and there are no plans to publish additional 2.x releases. It should be straightforward to experiment with the code snippets I posted previously or the proposed changes in this PR. That should work with any version of aiobotocore, including 2.x releases. |
|
Home Assistant is currently using aiobotocore 2.21.1 so it is probably good anyhow to check whether it is possible to upgrade to the latest version. |
Possibly, but that won't change anything: You still need to duplicate my proposed cache warming code, if you want to test it. So far, it's only contained in this pending PR and we are awaiting community feedback before we agree on a way to move forward. |
|
I applied the change manually to aiobotocore 2.21.1 and preload the caches. session = AioSession()
await hass.async_add_executor_job(session.warm_up_loader_caches, "s3")Some detections like the |
Sounds to me like a confirmation that the proposed approach works in the HA use case. |
Is there also a way to get rid of the SSL one? ssl_context.load_verify_locations(ca_certs, None, None) |
Glancing at the stacktrace you shared, I don't see how that would be related to aiobotocore. |
It seems to be triggered by: await cast(Any, client).head_bucket(Bucket=entry.data[CONF_BUCKET])Which is part of aiobotocore. |
Unblocking setup of the SSL context is another can of worms. This is out of scope of this PR. Please create a separate issue. |
|
the SSL context thing sounds like it could be easily handled by my version where we have an executor. sorry haven't had a chance to review the comments across all the issues yet in case this was already discussed, will try to catch up soon |
It would be great if there is already a solution in place that could work. |
95b54a2 to
42d671d
Compare
42d671d to
bbc0134
Compare
Description of Change
Support warm-up of loader caches
Assumptions
Replace this text with any assumptions made (if any)
Checklist for All Submissions
Checklist when updating botocore and/or aiohttp versions