Call managers instead of the store from APIs#942
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Pull request overview
This PR removes the admin API’s direct dependency on the persistent store by moving stats/accessors to the appropriate domain managers (accounts, contracts, hosts, slabs, subscriber), aligning with Issue #938.
Changes:
- Refactors admin API to fetch stats/state via managers instead of an
admin.Storeinterface. - Introduces/relocates stats types and thin manager methods for accounts/contracts/hosts/slabs and subscriber state.
- Updates Postgres store return types and adjusts related tests and wiring (cmd + testutils).
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| testutils/indexer.go | Updates admin API construction to pass managers + subscriber instead of store. |
| cmd/indexd/run.go | Updates production wiring for new admin.NewAPI signature. |
| api/admin/admin.go | Removes Store interface usage; routes stats/state through managers/subscriber. |
| api/admin/types.go | Replaces admin-local stats structs with types sourced from domain packages. |
| api/admin/prometheus.go | (Indirectly impacted) Continues to provide Prometheus marshalling via admin response wrapper types. |
| subscriber/subscriber.go | Exposes LastScannedIndex for admin state/metrics without passing store. |
| accounts/stats.go | Adds account/connect-key/app stats types + AccountManager accessors. |
| accounts/manager.go | Extends accounts store interface with stats methods. |
| contracts/stats.go | Adds contracts stats type + ContractManager.ContractsStats accessor. |
| contracts/manager.go | Extends contracts store interface with stats + delete methods; adds manager passthrough. |
| hosts/stats.go | Adds aggregated host stats type + HostManager.AggregatedHostStats accessor. |
| hosts/manager.go | Extends hosts store interface with aggregated stats method. |
| slabs/stats.go | Adds sectors/slabs stats type + SlabManager.SectorStats accessor. |
| slabs/manager.go | Extends slabs store interface and adds manager passthrough for slab/object queries + stats. |
| persist/postgres/stats.go | Changes stats return types to domain types (accounts/hosts/slabs). |
| persist/postgres/stats_test.go | Updates tests to use accounts.AppStats instead of admin types. |
| persist/postgres/contracts.go | Changes contracts stats return type to contracts.ContractsStats. |
| persist/postgres/contracts_test.go | Updates expected stats type to contracts.ContractsStats. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
peterjan
left a comment
There was a problem hiding this comment.
All our managers accept a context parameter but most don't do anything with it because our store methods don't take a context. We should consider removing all of those from the managers as well
ChrisSchinnerl
left a comment
There was a problem hiding this comment.
Lgtm. It doesn't fix the "silliness" as we have somehow started to call it but that's fine imo. It gets rid of the store fields and that's really the point of this PR.
Close #938