Skip to content

Commit 4929337

Browse files
committed
fix(config): produce annotated config.toml during legacy migration and
fix config source override order
1 parent dc01be1 commit 4929337

File tree

8 files changed

+130
-16
lines changed

8 files changed

+130
-16
lines changed

tests/config/test_legacy.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
import importlib
44

5+
import pytest
6+
57
from tidy3d.config.__init__ import get_manager, reload_config
68

79

@@ -14,9 +16,11 @@ def test_legacy_logging_level(config_manager):
1416

1517
def test_env_switch(config_manager):
1618
config_module = importlib.import_module("tidy3d.config.__init__")
17-
config_module.Env.dev.active()
19+
with pytest.warns(DeprecationWarning, match="tidy3d.config.Env"):
20+
config_module.Env.dev.active()
1821
assert get_manager().profile == "dev"
19-
config_module.Env.set_current(config_module.Env.prod)
22+
with pytest.warns(DeprecationWarning, match="tidy3d.config.Env"):
23+
config_module.Env.set_current(config_module.Env.prod)
2024
assert get_manager().profile == "prod"
2125

2226

tests/config/test_loader.py

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@
66
from pydantic import Field
77

88
from tidy3d.config import get_manager, reload_config
9+
from tidy3d.config import loader as config_loader
910
from tidy3d.config import registry as config_registry
11+
from tidy3d.config.legacy import finalize_legacy_migration
12+
from tidy3d.config.loader import migrate_legacy_config
1013
from tidy3d.config.sections import ConfigSection
1114
from tidy3d.web.cli.app import tidy3d_cli
1215

@@ -32,7 +35,7 @@ def test_save_includes_descriptions(config_manager, mock_config_dir):
3235
manager.save(include_defaults=True)
3336

3437
content = _config_path(mock_config_dir).read_text(encoding="utf-8")
35-
assert "# Web/HTTP configuration." in content
38+
assert "Lowest logging level that will be emitted." in content
3639

3740

3841
def test_preserves_user_comments(config_manager, mock_config_dir):
@@ -41,10 +44,7 @@ def test_preserves_user_comments(config_manager, mock_config_dir):
4144

4245
config_path = _config_path(mock_config_dir)
4346
text = config_path.read_text(encoding="utf-8")
44-
text = text.replace(
45-
"Web/HTTP configuration.",
46-
"user-modified comment",
47-
)
47+
text = text.replace("Lowest logging level that will be emitted.", "user-modified comment")
4848
config_path.write_text(text, encoding="utf-8")
4949

5050
reload_config(profile="default")
@@ -53,7 +53,7 @@ def test_preserves_user_comments(config_manager, mock_config_dir):
5353

5454
updated = config_path.read_text(encoding="utf-8")
5555
assert "user-modified comment" in updated
56-
assert "Web/HTTP configuration." not in updated
56+
assert "Lowest logging level that will be emitted." not in updated
5757

5858

5959
def test_profile_preserves_comments(config_manager, mock_config_dir):
@@ -118,7 +118,7 @@ class CLIPlugin(ConfigSection):
118118
assert result.exit_code == 0, result.output
119119

120120
config_text = _config_path(mock_config_dir).read_text(encoding="utf-8")
121-
assert "Web/HTTP configuration." in config_text
121+
assert "Lowest logging level that will be emitted." in config_text
122122
assert "[web]" in config_text
123123
assert "secret" not in config_text
124124
assert not profiles_dir.exists()
@@ -143,8 +143,58 @@ class CommentPlugin(ConfigSection):
143143
manager = get_manager()
144144
manager.save(include_defaults=True)
145145
content = _config_path(mock_config_dir).read_text(encoding="utf-8")
146-
assert "Comment plugin configuration." in content
147146
assert "Plugin knob description." in content
148147
finally:
149148
config_registry._SECTIONS.pop("plugins.comment_test", None)
150149
reload_config(profile="default")
150+
151+
152+
def test_finalize_legacy_migration_promotes_flat_file(tmp_path):
153+
canonical_dir = tmp_path / "canonical"
154+
canonical_dir.mkdir()
155+
legacy_file = canonical_dir / "config"
156+
legacy_file.write_text('apikey = "legacy-key"\n', encoding="utf-8")
157+
extra_file = canonical_dir / "extra.txt"
158+
extra_file.write_text("keep", encoding="utf-8")
159+
160+
finalize_legacy_migration(canonical_dir)
161+
162+
config_toml = canonical_dir / "config.toml"
163+
assert config_toml.exists()
164+
content = config_toml.read_text(encoding="utf-8")
165+
assert "[web]" in content
166+
assert "[logging]" in content
167+
assert "Lowest logging level that will be emitted." in content
168+
assert "legacy-key" in content
169+
assert not legacy_file.exists()
170+
assert extra_file.exists()
171+
assert extra_file.read_text(encoding="utf-8") == "keep"
172+
173+
174+
def test_migrate_legacy_config_promotes_structured_config(tmp_path, monkeypatch):
175+
legacy_dir = tmp_path / "legacy"
176+
legacy_dir.mkdir()
177+
legacy_file = legacy_dir / "config"
178+
legacy_file.write_text('apikey = "legacy-key"\n', encoding="utf-8")
179+
(legacy_dir / "extra.txt").write_text("keep", encoding="utf-8")
180+
181+
canonical_dir = tmp_path / "canonical"
182+
183+
monkeypatch.setattr(config_loader, "legacy_config_directory", lambda: legacy_dir)
184+
monkeypatch.setattr(config_loader, "canonical_config_directory", lambda: canonical_dir)
185+
186+
destination = migrate_legacy_config()
187+
188+
assert destination == canonical_dir
189+
config_toml = canonical_dir / "config.toml"
190+
assert config_toml.exists()
191+
content = config_toml.read_text(encoding="utf-8")
192+
assert "[web]" in content
193+
assert "[logging]" in content
194+
assert "Lowest logging level that will be emitted." in content
195+
assert "legacy-key" in content
196+
assert not (canonical_dir / "config").exists()
197+
extra_file = canonical_dir / "extra.txt"
198+
assert extra_file.exists()
199+
assert extra_file.read_text(encoding="utf-8") == "keep"
200+
assert legacy_dir.exists()

tests/config/test_manager.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,19 @@ def test_runtime_isolated_per_profile(config_manager):
2828
assert config_manager.get_section("web").timeout == 45
2929

3030

31-
def test_environment_variable_precedence(monkeypatch, config_manager):
31+
def test_runtime_overrides_env(monkeypatch, config_manager):
3232
monkeypatch.setenv("TIDY3D_LOGGING__LEVEL", "WARNING")
3333
config_manager.switch_profile(config_manager.profile)
3434
config_manager.update_section("logging", level="DEBUG")
3535
logging_section = config_manager.get_section("logging")
36-
# env var should still take precedence
36+
# runtime change should override the environment variable
37+
assert logging_section.level == "DEBUG"
38+
39+
40+
def test_env_applies_without_runtime_override(monkeypatch, config_manager):
41+
monkeypatch.setenv("TIDY3D_LOGGING__LEVEL", "WARNING")
42+
config_manager.switch_profile(config_manager.profile)
43+
logging_section = config_manager.get_section("logging")
3744
assert logging_section.level == "WARNING"
3845

3946

tidy3d/config/legacy.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import os
1010
import ssl
11+
import warnings
1112
from pathlib import Path
1213
from typing import Any, Optional
1314

@@ -19,6 +20,12 @@
1920
from .profiles import BUILTIN_PROFILES
2021

2122

23+
def _warn_env_deprecated() -> None:
24+
message = "'tidy3d.config.Env' is deprecated; use 'config.switch_profile(...)' instead."
25+
warnings.warn(message, DeprecationWarning, stacklevel=3)
26+
log.warning(message, log_once=True)
27+
28+
2229
class LegacyConfigWrapper:
2330
"""Provide attribute-level compatibility with the legacy config module."""
2431

@@ -169,6 +176,7 @@ def manager(self) -> Optional[ConfigManager]:
169176
return self._manager
170177

171178
def active(self) -> None:
179+
_warn_env_deprecated()
172180
if self._manager is not None and self._manager.profile != self._name:
173181
self._manager.switch_profile(self._name)
174182

@@ -296,6 +304,7 @@ def current(self) -> LegacyEnvironmentConfig:
296304
return self._current
297305

298306
def set_current(self, env_config: LegacyEnvironmentConfig) -> None:
307+
_warn_env_deprecated()
299308
key = normalize_profile_name(env_config.name)
300309
if env_config.manager is self._manager:
301310
if self._manager.profile != key:
@@ -377,5 +386,42 @@ def load_legacy_flat_config(config_dir: Path) -> dict[str, Any]:
377386
"LegacyConfigWrapper",
378387
"LegacyEnvironment",
379388
"LegacyEnvironmentConfig",
389+
"finalize_legacy_migration",
380390
"load_legacy_flat_config",
381391
]
392+
393+
394+
def finalize_legacy_migration(config_dir: Path) -> None:
395+
"""Promote a copied legacy configuration tree into the structured format.
396+
397+
Parameters
398+
----------
399+
config_dir : Path
400+
Destination directory (typically the canonical config location).
401+
"""
402+
403+
legacy_data = load_legacy_flat_config(config_dir)
404+
405+
from .manager import ConfigManager # local import to avoid circular dependency
406+
407+
manager = ConfigManager(profile="default", config_dir=config_dir)
408+
config_path = config_dir / "config.toml"
409+
for section, values in legacy_data.items():
410+
if isinstance(values, dict):
411+
manager.update_section(section, **values)
412+
try:
413+
manager.save(include_defaults=True)
414+
except Exception:
415+
if config_path.exists():
416+
try:
417+
config_path.unlink()
418+
except Exception:
419+
pass
420+
raise
421+
422+
legacy_flat_path = config_dir / "config"
423+
if legacy_flat_path.exists():
424+
try:
425+
legacy_flat_path.unlink()
426+
except Exception as exc:
427+
log.warning(f"Failed to remove legacy configuration file '{legacy_flat_path}': {exc}")

tidy3d/config/loader.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,10 @@ def migrate_legacy_config(*, overwrite: bool = False, remove_legacy: bool = Fals
339339
canonical_dir.parent.mkdir(parents=True, exist_ok=True)
340340
shutil.copytree(legacy_dir, canonical_dir, dirs_exist_ok=overwrite)
341341

342+
from .legacy import finalize_legacy_migration # local import to avoid circular dependency
343+
344+
finalize_legacy_migration(canonical_dir)
345+
342346
if remove_legacy:
343347
shutil.rmtree(legacy_dir)
344348

tidy3d/config/manager.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ def _reload(self) -> None:
318318
self._raw_tree = deep_merge(self._builtin_data, self._base_data, self._profile_data)
319319

320320
runtime = deepcopy(self._runtime_overrides.get(self._profile, {}))
321-
effective = deep_merge(self._raw_tree, runtime, self._env_overrides)
321+
effective = deep_merge(self._raw_tree, self._env_overrides, runtime)
322322
self._effective_tree = effective
323323
self._build_models()
324324

tidy3d/config/serializer.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ def _apply_value(
123123
if key in container:
124124
container[key] = table
125125
else:
126-
if description:
127-
container.add(tomlkit.comment(description))
126+
if isinstance(container, tomlkit.TOMLDocument) and len(container) > 0:
127+
container.add(tomlkit.nl())
128128
container.add(key, table)
129129
return
130130

tidy3d/web/cli/app.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,10 @@ def config_migrate(overwrite: bool, delete_legacy: bool) -> None:
197197
if delete_legacy:
198198
click.echo("The legacy '~/.tidy3d' directory was removed.")
199199
else:
200-
click.echo(f"The legacy directory remains at '{legacy_dir}'.")
200+
click.echo(
201+
f"The legacy directory remains at '{legacy_dir}'. "
202+
"Remove it after confirming the new configuration works, or rerun with '--delete-legacy'."
203+
)
201204

202205

203206
@click.group()

0 commit comments

Comments
 (0)