perf(clusters): avoid eager node loading in list operations#184
Open
perf(clusters): avoid eager node loading in list operations#184
Conversation
There was a problem hiding this comment.
Pull request overview
This PR optimizes cluster list operations by introducing a lazy-loading pattern to avoid eagerly loading cluster nodes. For a setup with 4 clusters, this reduces the CLI execution time from ~5s to ~1s by eliminating 2-3 API calls per cluster during list operations.
Changes:
- Introduced
ClusterSummarybase model (without nodes) thatClusterextends, implementing the list-projection vs detail-projection pattern - Updated
ClusterRepository.list()to returnList[ClusterSummary]instead ofList[Cluster], eliminating calls to_load_cluster_nodes() - Updated all call sites (CLI apps, service layer, workspace provider) to use
ClusterSummaryfor list operations whileget()operations still return fullClusterwith nodes
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| exls/clusters/core/domain.py | Added ClusterSummary base model and made Cluster extend it with nodes field |
| exls/clusters/core/ports/repository.py | Updated list() return type to ClusterSummary |
| exls/clusters/adapters/adapter.py | Modified list() to create ClusterSummary objects without loading nodes |
| exls/clusters/core/service.py | Updated list_clusters() return type and _validate_cluster_status() parameter type |
| exls/clusters/app.py | Updated type annotations for cluster list and resolution operations |
| exls/services/app.py | Updated type annotation for cluster resolution in service commands |
| exls/workspaces/adapters/provider/clusters.py | Updated type annotation for cluster listing in workspace provider |
| tests/unit/clusters/test_clusters_service.py | Added ClusterSummary test fixture and updated list_clusters test expectations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Every cluster operation that called
list_clusters(), includingexls clusters listand the_resolve_cluster_id_callbackthat runs before every command with a cluster argument, eagerly loaded nodes for every cluster. For 5 clusters,exls clusters listmade ~16 API calls when it only needed 1.ClusterSummarybase model (without nodes) and makeClusterextend it, following the list-projection vs detail-projection pattern already used in the CLI appClusterRepository.list()now returnsList[ClusterSummary], skipping_load_cluster_nodes()entirely and therefore eliminates 2-3 API calls per cluster (get_cluster_nodes, list_nodes, get_cluster_resources)ClusterRepository.get()still returns fullClusterwith nodes, unchangedNotes for Reviewers
For a setup with e.g. 4 clusters, this reduces the time for listing the clusters with exls from >5s to ~1s.
Before:
After: