Skip to content

Commit 68b4ebd

Browse files
authored
Update migrations list command to show migrations that no longer exist in the codebase (#6299)
Fixes #2159 Running `snuba migrations list` will now show migrations that are in clickhouse but no longer in the codebase (e.g. because snuba was downgraded). # Testing Deleted migration in codebase to check that it shows up in the list. <img width="668" alt="image" src="https://github.com/user-attachments/assets/90f855b9-94af-416d-9452-0335c2abd8d6">
1 parent d789447 commit 68b4ebd

File tree

6 files changed

+63
-15
lines changed

6 files changed

+63
-15
lines changed

snuba/admin/clickhouse/migration_checks.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ def __init__(
100100
).get_migrations()
101101

102102
migration_statuses = {}
103-
for migration_id, status, _ in migrations:
103+
for migration_id, status, _, _ in migrations:
104104
migration_statuses[migration_id] = {
105105
"migration_id": migration_id,
106106
"status": status,

snuba/admin/views.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ def migrations_groups_list(group: str) -> Response:
208208
"status": status.value,
209209
"blocking": blocking,
210210
}
211-
for migration_id, status, blocking in runner_group_migrations
211+
for migration_id, status, blocking, _ in runner_group_migrations
212212
]
213213
),
214214
200,

snuba/cli/migrations.py

+7-3
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,10 @@ def list() -> None:
3737
setup_logging()
3838
check_clickhouse_connections(CLUSTERS)
3939
runner = Runner()
40-
for group, group_migrations in runner.show_all():
40+
for group, group_migrations in runner.show_all(include_nonexistent=True):
4141
readiness_state = get_group_readiness_state(group)
4242
click.echo(f"{group.value} (readiness_state: {readiness_state.value})")
43-
for migration_id, status, blocking in group_migrations:
43+
for migration_id, status, blocking, existing in group_migrations:
4444
symbol = {
4545
Status.COMPLETED: "X",
4646
Status.NOT_STARTED: " ",
@@ -53,7 +53,11 @@ def list() -> None:
5353
if status != Status.COMPLETED and blocking:
5454
blocking_text = " (blocking)"
5555

56-
click.echo(f"[{symbol}] {migration_id}{in_progress_text}{blocking_text}")
56+
existing_text = "" if existing else " (this migration no longer exists)"
57+
58+
click.echo(
59+
f"[{symbol}] {migration_id}{in_progress_text}{blocking_text}{existing_text}"
60+
)
5761

5862
click.echo()
5963

snuba/migrations/runner.py

+24-3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from collections import defaultdict
12
from datetime import datetime
23
from functools import partial
34
from typing import List, Mapping, MutableMapping, NamedTuple, Optional, Sequence, Tuple
@@ -64,6 +65,7 @@ class MigrationDetails(NamedTuple):
6465
migration_id: str
6566
status: Status
6667
blocking: bool
68+
exists: bool
6769

6870

6971
class Runner:
@@ -133,7 +135,7 @@ def force_overwrite_status(
133135
)
134136

135137
def show_all(
136-
self, groups: Optional[Sequence[str]] = None
138+
self, groups: Optional[Sequence[str]] = None, include_nonexistent: bool = False
137139
) -> List[Tuple[MigrationGroup, List[MigrationDetails]]]:
138140
"""
139141
Returns the list of migrations and their statuses for each group.
@@ -148,6 +150,9 @@ def show_all(
148150
migration_groups = get_active_migration_groups()
149151

150152
migration_status = self._get_migration_status(migration_groups)
153+
clickhouse_group_migrations = defaultdict(set)
154+
for group, migration_id in migration_status.keys():
155+
clickhouse_group_migrations[group].add(migration_id)
151156

152157
def get_status(migration_key: MigrationKey) -> Status:
153158
return migration_status.get(migration_key, Status.NOT_STARTED)
@@ -156,15 +161,31 @@ def get_status(migration_key: MigrationKey) -> Status:
156161
group_migrations: List[MigrationDetails] = []
157162
group_loader = get_group_loader(group)
158163

159-
for migration_id in group_loader.get_migrations():
164+
migration_ids = group_loader.get_migrations()
165+
for migration_id in migration_ids:
160166
migration_key = MigrationKey(group, migration_id)
161167
migration = group_loader.load_migration(migration_id)
162168
group_migrations.append(
163169
MigrationDetails(
164-
migration_id, get_status(migration_key), migration.blocking
170+
migration_id,
171+
get_status(migration_key),
172+
migration.blocking,
173+
True,
165174
)
166175
)
167176

177+
if include_nonexistent:
178+
non_existing_migrations = clickhouse_group_migrations.get(
179+
group, set()
180+
).difference(set(migration_ids))
181+
for migration_id in non_existing_migrations:
182+
migration_key = MigrationKey(group, migration_id)
183+
group_migrations.append(
184+
MigrationDetails(
185+
migration_id, get_status(migration_key), False, False
186+
)
187+
)
188+
168189
migrations.append((group, group_migrations))
169190

170191
return migrations

tests/admin/clickhouse_migrations/test_migration_checks.py

+10-7
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ def group_loader() -> GroupLoader:
2929

3030

3131
RUN_MIGRATIONS: Sequence[MigrationDetails] = [
32-
MigrationDetails("0001", Status.COMPLETED, True),
33-
MigrationDetails("0002", Status.NOT_STARTED, True),
34-
MigrationDetails("0003", Status.NOT_STARTED, True),
32+
MigrationDetails("0001", Status.COMPLETED, True, True),
33+
MigrationDetails("0002", Status.NOT_STARTED, True, True),
34+
MigrationDetails("0003", Status.NOT_STARTED, True, True),
3535
]
3636

3737

@@ -62,9 +62,9 @@ def test_status_checker_run(
6262

6363

6464
REVERSE_MIGRATIONS: Sequence[MigrationDetails] = [
65-
MigrationDetails("0001", Status.COMPLETED, True),
66-
MigrationDetails("0002", Status.IN_PROGRESS, True),
67-
MigrationDetails("0003", Status.NOT_STARTED, True),
65+
MigrationDetails("0001", Status.COMPLETED, True, True),
66+
MigrationDetails("0002", Status.IN_PROGRESS, True, True),
67+
MigrationDetails("0003", Status.NOT_STARTED, True, True),
6868
]
6969

7070

@@ -155,7 +155,10 @@ def test_run_migration_checks_and_policies(
155155
mock_policy = Mock()
156156
checker = mock_checker()
157157
mock_runner.show_all.return_value = [
158-
(MigrationGroup("events"), [MigrationDetails("0001", Status.COMPLETED, True)])
158+
(
159+
MigrationGroup("events"),
160+
[MigrationDetails("0001", Status.COMPLETED, True, True)],
161+
)
159162
]
160163

161164
mock_policy.can_run.return_value = policy_result[0]

tests/migrations/test_runner.py

+20
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,26 @@ def test_show_all_for_groups() -> None:
110110
assert all([migration.status == Status.COMPLETED for migration in migrations])
111111

112112

113+
@pytest.mark.clickhouse_db
114+
def test_show_all_nonexistent_migration() -> None:
115+
runner = Runner()
116+
assert all(
117+
[
118+
migration.status == Status.NOT_STARTED
119+
for (_, group_migrations) in runner.show_all()
120+
for migration in group_migrations
121+
]
122+
)
123+
runner.run_all(force=True)
124+
assert all(
125+
[
126+
migration.status == Status.COMPLETED
127+
for (_, group_migrations) in runner.show_all()
128+
for migration in group_migrations
129+
]
130+
)
131+
132+
113133
@pytest.mark.clickhouse_db
114134
def test_run_migration() -> None:
115135
runner = Runner()

0 commit comments

Comments
 (0)