fix: Cache loaded identities#479
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds in-process caching to the ICP identity loader so repeated identity loads (within a single CLI execution) reuse an already-loaded Arc<dyn Identity> instead of re-reading identity files / re-prompting for secrets.
Changes:
- Add an internal
Mutex<HashMap<IdentitySelection, Arc<dyn Identity>>>cache toidentity::Loaderand route construction through a newLoader::new(...)constructor. - Update
IdentitySelectionto deriveEq+Hashto support use as aHashMapkey. - Update context initialization to construct the loader via
Loader::new(...)rather than a struct literal.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/icp/src/identity/mod.rs | Introduces cached identity loading, constructor, and hashing support for IdentitySelection. |
| crates/icp/src/context/init.rs | Switches identity loader initialization to the new Loader::new(...) API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async fn load(&self, id: IdentitySelection) -> Result<Arc<dyn Identity>, LoadError> { | ||
| if let Some(cached) = self.cache.lock().unwrap().get(&id) { | ||
| return Ok(Arc::clone(cached)); | ||
| } |
There was a problem hiding this comment.
On cache hits this returns early without re-setting telemetry_data.identity_type, which can leave telemetry reflecting a previously loaded (different) identity selection if the loader is reused within the same process. Consider caching the computed IdentityStorageType alongside the Arc<dyn Identity> and calling telemetry_data.set_identity_type(...) even when serving from cache (or otherwise ensuring telemetry is updated per load() call).
No description provided.