Skip to content

fix(admin): Allow listing migrations without every storage set configured - #8412

Open
phacops wants to merge 1 commit into
masterfrom
fix/unregistered-storage-set-keys
Open

fix(admin): Allow listing migrations without every storage set configured#8412
phacops wants to merge 1 commit into
masterfrom
fix/unregistered-storage-set-keys

Conversation

@phacops

@phacops phacops commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

GET /migrations/groups imported every historical migration to build the list. Those modules reference StorageSetKey names that only existed if this process's cluster config had registered them, so a missing generic_metrics_distributions mapping 500'd the whole page.

StorageSetKey.FOO is now always constructible. get_cluster() still raises if that set isn't mapped, so you cannot run DDL against a cluster that isn't there.

Fixes SNUBA-CKE

Listing migrations imports historical modules that name storage sets
absent from this process's cluster config, which 500'd /migrations/groups.

Keep identifiers constructible; get_cluster() still fails if unmapped.

Fixes SNUBA-CKE
@phacops
phacops marked this pull request as ready for review August 28, 2026 19:00
@phacops
phacops requested a review from a team as a code owner August 28, 2026 19:00
@pbhandari

Copy link
Copy Markdown
Contributor

If we have a small list of legacy storage sets, is that a reasonable approach?

@MeredithAnya

Copy link
Copy Markdown
Member

If we have a small list of legacy storage sets, is that a reasonable approach?

yeah i was about to suggest having DEPRECATED_STORAGE_SETS_KEYS, cause this seems too permissive to me

@phacops

phacops commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

If we have a small list of legacy storage sets, is that a reasonable approach?

yeah i was about to suggest having DEPRECATED_STORAGE_SETS_KEYS, cause this seems too permissive to me

Why do we care about controlling all this? If the storage set is not configured, we don't see the migration groups in the list and that's it. We don't need to have a controlled number of migration sets not in the list?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants