Skip to content

Commit 60eef76

Browse files
committed
🐛 Materialize migration iterables (#7491)
`config_can_be_downgraded`, `upgrade_config` and `downgrade_config` search the migrations iterable repeatedly, so a one-shot iterator (e.g. a generator) was consumed after the first step and later lookups wrongly failed. Latent because the default `MIGRATIONS` is a tuple. Materialize to a tuple in each helper and add generator-based coverage over a multi-step upgrade/downgrade.
1 parent 7f36513 commit 60eef76

2 files changed

Lines changed: 33 additions & 3 deletions

File tree

src/aiida/manage/configuration/migrations/migrations.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,8 @@ def config_can_be_downgraded(
517517
"""
518518
target = CURRENT_CONFIG_VERSION if target is None else target
519519
current = get_current_version(config)
520+
# Materialize so the iterable can be searched repeatedly, even if a one-shot iterator was passed.
521+
migrations = tuple(migrations)
520522

521523
if current <= target or current > MAXIMUM_DOWNGRADE_CONFIG_VERSION:
522524
return False
@@ -544,6 +546,8 @@ def upgrade_config(
544546
:return: the migrated configuration dictionary
545547
"""
546548
current = get_current_version(config)
549+
# Materialize so the iterable can be searched repeatedly, even if a one-shot iterator was passed.
550+
migrations = tuple(migrations)
547551
used = []
548552
while current < target:
549553
current = get_current_version(config)
@@ -572,6 +576,8 @@ def downgrade_config(
572576
:return: the migrated configuration dictionary
573577
"""
574578
current = get_current_version(config)
579+
# Materialize so the iterable can be searched repeatedly, even if a one-shot iterator was passed.
580+
migrations = tuple(migrations)
575581
if current > MAXIMUM_DOWNGRADE_CONFIG_VERSION:
576582
msg = (
577583
f'Cannot downgrade configuration version {current}: this AiiDA version can only downgrade configuration '

tests/manage/configuration/migrations/test_migrations.py

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,19 @@ def test_config_needs_migrating_incompatible_version(monkeypatch):
9292

9393

9494
def test_config_can_be_downgraded():
95-
"""Test detecting whether a configuration can be downgraded by this AiiDA version."""
95+
"""Test detecting whether a configuration can be downgraded by this AiiDA version.
96+
97+
The migrations are passed as one-shot generators over a multi-step chain to ensure the iterable is materialized
98+
before being searched repeatedly (a consumed iterator would make the second lookup wrongly fail).
99+
"""
96100
assert config_can_be_downgraded(
97101
{'CONFIG_VERSION': {'CURRENT': MAXIMUM_DOWNGRADE_CONFIG_VERSION, 'OLDEST_COMPATIBLE': 0}},
98-
target=MAXIMUM_DOWNGRADE_CONFIG_VERSION - 1,
102+
target=MAXIMUM_DOWNGRADE_CONFIG_VERSION - 2,
103+
migrations=(m for m in MIGRATIONS),
99104
)
100105
assert not config_can_be_downgraded(
101-
{'CONFIG_VERSION': {'CURRENT': MAXIMUM_DOWNGRADE_CONFIG_VERSION + 1, 'OLDEST_COMPATIBLE': 0}}
106+
{'CONFIG_VERSION': {'CURRENT': MAXIMUM_DOWNGRADE_CONFIG_VERSION + 1, 'OLDEST_COMPATIBLE': 0}},
107+
migrations=(m for m in MIGRATIONS),
102108
)
103109

104110

@@ -115,6 +121,24 @@ def test_migrate_full(load_config_sample, monkeypatch):
115121
assert config_migrated == config_target
116122

117123

124+
def test_migrate_full_downgrade(load_config_sample, monkeypatch):
125+
"""Test the full config downgrade, as a counterpart to :func:`test_migrate_full`.
126+
127+
The oldest sample is upgraded to the latest version and then downgraded all the way back, exercising the full
128+
migration chain in both directions. Downgrades are lossy by design (e.g. a profile UUID added on upgrade is
129+
intentionally kept), so this asserts the versions reached rather than round-trip equality. The migrations are
130+
passed as one-shot generators, which additionally guards that the iterable is materialized before the repeated
131+
per-step lookups (a consumed iterator would stop finding migrations after the first step).
132+
"""
133+
monkeypatch.setattr(uuid, 'uuid4', lambda: uuid.UUID(hex='0' * 32))
134+
135+
upgraded = upgrade_config(load_config_sample('input/0.json'), 10, migrations=(m for m in MIGRATIONS))
136+
assert upgraded['CONFIG_VERSION']['CURRENT'] == 10
137+
138+
downgraded = downgrade_config(upgraded, 0, migrations=(m for m in MIGRATIONS))
139+
assert downgraded['CONFIG_VERSION']['CURRENT'] == 0
140+
141+
118142
@pytest.mark.parametrize('initial, target', ((m.down_revision, m.up_revision) for m in MIGRATIONS))
119143
def test_migrate_individual(load_config_sample, initial, target, monkeypatch):
120144
"""Test the individual config migrations."""

0 commit comments

Comments
 (0)