Skip to content

Commit 830fc45

Browse files
johnnygrecojohnnygreco
committed
harden versioned plugin installs
- Preserve catalog requirement constraints for versioned installs - Remove stale install-plan metadata fields - Expand parser, uv, controller, and local-catalog dry-run coverage
1 parent 32b887d commit 830fc45

9 files changed

Lines changed: 318 additions & 54 deletions

File tree

‎packages/data-designer/src/data_designer/cli/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ data-designer plugin install github --yes
345345
# Install a specific plugin package version from the catalog package index
346346
data-designer plugin install github --version 0.1.0 --yes
347347
348-
# Preview without changing the current environment. Exits 1 if compatibility would block install.
348+
# Preview a specific package version without changing the current environment
349349
data-designer plugin install github==0.1.0 --dry-run
350350
351351
# Uninstall a plugin package and verify its runtime entry-point metadata is removed

‎packages/data-designer/src/data_designer/cli/commands/plugin.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ def info_command(
8686
help="Fetch the catalog even when a fresh cache entry exists.",
8787
),
8888
) -> None:
89-
"""Show metadata, compatibility, docs, and install plan for one plugin package."""
89+
"""Show metadata, compatibility, docs, and install strategy for one plugin package."""
9090
controller = PluginCatalogController(DATA_DESIGNER_HOME)
9191
controller.run_info(
9292
package,

‎packages/data-designer/src/data_designer/cli/main.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ def _is_version_request(args: list[str]) -> bool:
160160
"info": {
161161
"module": f"{_CMD}.plugin",
162162
"attr": "info_command",
163-
"help": "Show plugin package metadata and install plan",
163+
"help": "Show plugin package metadata and install strategy",
164164
},
165165
"install": {
166166
"module": f"{_CMD}.plugin",
@@ -175,7 +175,7 @@ def _is_version_request(args: list[str]) -> bool:
175175
"installed": {
176176
"module": f"{_CMD}.plugin",
177177
"attr": "installed_command",
178-
"help": "List installed runtime plugins and their packages",
178+
"help": "List installed plugin packages and their runtime plugins",
179179
},
180180
}
181181
),

‎packages/data-designer/src/data_designer/cli/plugin_catalog.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,13 +217,11 @@ class InstallPlan:
217217
"""Resolved package-manager command for installing one plugin package."""
218218

219219
package_name: str
220-
source_description: str
221220
command: list[str]
222221
manager: str
223222
catalog_alias: str
224223
requirement: str | None = None
225224
source_warning: str | None = None
226-
data_designer_protection: str | None = None
227225
data_designer_version: str | None = None
228226
command_stdin: str | None = None
229227
temporary_file: InstallCommandTemporaryFile | None = None

‎packages/data-designer/src/data_designer/cli/services/plugin_install_service.py‎

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -94,25 +94,23 @@ def build_install_plan(
9494
active_virtualenv=self._active_virtualenv,
9595
)
9696
data_designer_versions = _installed_data_designer_distribution_versions()
97-
protection_args, data_designer_protection, command_stdin, temporary_file = _data_designer_protection_args(
97+
protection_args, command_stdin, temporary_file = _data_designer_protection_args(
9898
target.mode,
9999
data_designer_versions,
100100
)
101-
install_args, install_requirement, source_description, source_warning = _install_args_for_entry(
101+
install_args, install_requirement, source_warning = _install_args_for_entry(
102102
entry,
103103
target,
104104
version_specifier=version_specifier,
105105
)
106106
command = _base_command(target) + protection_args + install_args
107107
return InstallPlan(
108108
package_name=entry.package.name,
109-
source_description=source_description,
110109
command=command,
111110
manager=target.manager,
112111
catalog_alias=catalog.alias,
113112
requirement=install_requirement,
114113
source_warning=_combine_warnings(target.warning, source_warning),
115-
data_designer_protection=data_designer_protection,
116114
data_designer_version=data_designer_versions[DATA_DESIGNER_DISTRIBUTION_NAME],
117115
command_stdin=command_stdin,
118116
temporary_file=temporary_file,
@@ -520,13 +518,10 @@ def _installed_data_designer_distribution_versions() -> dict[str, str]:
520518
def _data_designer_protection_args(
521519
mode: str,
522520
versions: dict[str, str],
523-
) -> tuple[list[str], str, str | None, InstallCommandTemporaryFile | None]:
524-
data_designer_version = versions[DATA_DESIGNER_DISTRIBUTION_NAME]
521+
) -> tuple[list[str], str | None, InstallCommandTemporaryFile | None]:
525522
if mode == "uv-environment":
526523
return (
527524
["--constraint", "-"],
528-
f"using installed {DATA_DESIGNER_DISTRIBUTION_NAME} {data_designer_version}; "
529-
"uv will keep Data Designer packages pinned",
530525
_data_designer_constraint_text(versions),
531526
None,
532527
)
@@ -540,15 +535,12 @@ def _data_designer_protection_args(
540535
for item in ("--no-install-package", distribution_name)
541536
],
542537
],
543-
f"using installed {DATA_DESIGNER_DISTRIBUTION_NAME} {data_designer_version}; "
544-
"uv will not install Data Designer packages",
545538
None,
546539
None,
547540
)
548541

549542
return (
550543
["--constraint", DATA_DESIGNER_CONSTRAINT_PLACEHOLDER],
551-
f"pinned installed Data Designer packages; {DATA_DESIGNER_DISTRIBUTION_NAME} {data_designer_version}",
552544
None,
553545
_data_designer_constraint_file(versions),
554546
)
@@ -588,30 +580,28 @@ def _install_args_for_entry(
588580
target: _InstallTarget,
589581
*,
590582
version_specifier: str | None,
591-
) -> tuple[list[str], str, str, str | None]:
583+
) -> tuple[list[str], str, str | None]:
592584
requirement = _install_requirement_for_entry(entry, version_specifier=version_specifier)
593585
index_url = entry.install.index_url
594586
if target.mode == "uv-project":
595587
args = ["--raw"] if index_url is None and _requirement_is_direct_reference(requirement) else []
596588
if index_url is not None:
597589
args.extend(["--index", index_url])
598590
args.append(requirement)
599-
return args, requirement, _source_description(requirement, index_url), None
591+
return args, requirement, None
600592

601593
if index_url is None:
602-
return [requirement], requirement, requirement, None
594+
return [requirement], requirement, None
603595

604596
if target.manager == "uv":
605597
return (
606598
["--default-index", PYPI_SIMPLE_INDEX_URL, "--index", index_url, requirement],
607599
requirement,
608-
f"{requirement} via {index_url}",
609600
None,
610601
)
611602
return (
612603
["--extra-index-url", index_url, requirement],
613604
requirement,
614-
f"{requirement} via {index_url}",
615605
PIP_EXTRA_INDEX_SOURCE_WARNING,
616606
)
617607

@@ -633,14 +623,10 @@ def _install_requirement_for_entry(entry: PluginCatalogEntry, *, version_specifi
633623
)
634624

635625
extras = f"[{','.join(sorted(parsed_requirement.extras))}]" if parsed_requirement.extras else ""
626+
specifiers = [specifier for specifier in (str(parsed_requirement.specifier), version_specifier) if specifier]
627+
combined_specifier = ",".join(specifiers)
636628
marker = f"; {parsed_requirement.marker}" if parsed_requirement.marker is not None else ""
637-
return f"{entry.package.name}{extras}{version_specifier}{marker}"
638-
639-
640-
def _source_description(requirement: str, index_url: str | None) -> str:
641-
if index_url is None:
642-
return requirement
643-
return f"{requirement} via {index_url}"
629+
return f"{entry.package.name}{extras}{combined_specifier}{marker}"
644630

645631

646632
def _combine_warnings(*warnings: str | None) -> str | None:

‎packages/data-designer/tests/cli/controllers/test_plugin_catalog_controller.py‎

Lines changed: 130 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33

44
from __future__ import annotations
55

6+
import json
67
from pathlib import Path
8+
from types import SimpleNamespace
79
from unittest.mock import MagicMock, call, patch
810

911
import pytest
@@ -291,10 +293,7 @@ def test_run_info_renders_package_metadata_with_nested_runtime_plugins(
291293
controller.catalog_service.get_catalog.return_value = catalog
292294
controller.catalog_service.get_package_entries.return_value = package_entries
293295
controller.catalog_service.evaluate_compatibility.return_value = CompatibilityResult(True, [])
294-
controller.install_service.build_install_plan.return_value = _plan(
295-
catalog,
296-
data_designer_protection="uv will keep Data Designer packages pinned",
297-
)
296+
controller.install_service.build_install_plan.return_value = _plan(catalog)
298297

299298
controller.run_info("text-transform", catalog_alias="local")
300299

@@ -455,7 +454,7 @@ def test_run_install_dry_run_renders_plan_without_installing(
455454
) -> None:
456455
entry = _entry()
457456
catalog = _catalog()
458-
plan = _plan(catalog, data_designer_protection="pinned installed Data Designer packages; data-designer 0.5.10")
457+
plan = _plan(catalog)
459458
controller.catalog_service.get_catalog.return_value = catalog
460459
controller.catalog_service.get_package_entries.return_value = [entry]
461460
controller.catalog_service.evaluate_compatibility.return_value = CompatibilityResult(True, [])
@@ -612,6 +611,89 @@ def test_run_install_versioned_dry_run_warns_instead_of_blocking_on_catalog_comp
612611
mock_print_success.assert_any_call("Dry run complete; no changes made")
613612

614613

614+
@patch("data_designer.cli.controllers.plugin_catalog_controller.console")
615+
@patch("data_designer.cli.controllers.plugin_catalog_controller.print_success")
616+
@patch("data_designer.cli.controllers.plugin_catalog_controller.print_warning")
617+
def test_run_install_versioned_real_install_warns_instead_of_blocking_on_catalog_compatibility(
618+
mock_print_warning: MagicMock,
619+
mock_print_success: MagicMock,
620+
mock_console: MagicMock,
621+
controller: PluginCatalogController,
622+
) -> None:
623+
entry = _entry(package_name="data-designer-github")
624+
catalog = _catalog()
625+
plan = _plan(catalog, package_name="data-designer-github", requirement="data-designer-github==0.1.0")
626+
controller.catalog_service.get_catalog.return_value = catalog
627+
controller.catalog_service.get_package_entries.return_value = [entry]
628+
controller.catalog_service.evaluate_compatibility.return_value = CompatibilityResult(
629+
False,
630+
["Data Designer 0.5.7 does not satisfy >=99.0"],
631+
)
632+
controller.install_service.build_install_plan.return_value = plan
633+
controller.install_service.verify_entry_points.return_value = True
634+
635+
controller.run_install("github==0.1.0", catalog_alias="local", yes=True)
636+
637+
controller.install_service.build_install_plan.assert_called_once_with(
638+
entry,
639+
catalog,
640+
manager="auto",
641+
version_specifier="==0.1.0",
642+
)
643+
controller.install_service.install.assert_called_once_with(plan)
644+
controller.install_service.verify_entry_points.assert_called_once_with([entry])
645+
mock_print_warning.assert_called_once_with(
646+
"Catalog compatibility metadata may describe the catalog's default package version, not the requested version. "
647+
"Data Designer packages remain pinned during install; the package manager will fail if the requested plugin "
648+
"version cannot use the installed Data Designer version."
649+
)
650+
mock_print_success.assert_called_once_with(
651+
"Plugin package 'data-designer-github' installed and runtime entry points loaded"
652+
)
653+
assert mock_console.print.call_count >= 1
654+
655+
656+
def test_run_install_versioned_dry_run_uses_local_catalog_and_real_services(tmp_path: Path) -> None:
657+
catalog_file = tmp_path / "plugins.json"
658+
catalog_file.write_text(
659+
json.dumps(
660+
_catalog_payload(
661+
package_name="data-designer-github",
662+
runtime_plugin_name="github",
663+
install_requirement="data-designer-github",
664+
)
665+
),
666+
encoding="utf-8",
667+
)
668+
669+
with (
670+
patch("data_designer.cli.controllers.plugin_catalog_controller.console") as mock_console,
671+
patch("data_designer.cli.controllers.plugin_catalog_controller.print_success") as mock_print_success,
672+
patch(
673+
"data_designer.cli.services.plugin_catalog_service.importlib.metadata.version",
674+
return_value="0.5.10",
675+
),
676+
patch(
677+
"data_designer.cli.services.plugin_install_service.importlib.metadata.version",
678+
return_value="0.5.10",
679+
),
680+
patch(
681+
"data_designer.cli.services.plugin_install_service.subprocess.run",
682+
return_value=SimpleNamespace(returncode=0, stdout="pip 24.0\n", stderr=""),
683+
),
684+
):
685+
plugin_controller = PluginCatalogController(tmp_path / "data-designer-home")
686+
plugin_controller.catalog_service.add_catalog("local", str(catalog_file))
687+
688+
plugin_controller.run_install("github==0.1.0", catalog_alias="local", manager="pip", dry_run=True, refresh=True)
689+
690+
mock_console.print.assert_any_call(" Runtime plugins: [bold]github (processor)[/bold]")
691+
mock_console.print.assert_any_call(" Requirement: [bold]data-designer-github==0.1.0[/bold]")
692+
mock_console.print.assert_any_call(" Install strategy: [bold]pip install[/bold]")
693+
mock_console.print.assert_any_call(" data-designer version: [bold]0.5.10[/bold]")
694+
mock_print_success.assert_any_call("Dry run complete; no changes made")
695+
696+
615697
@patch("data_designer.cli.controllers.plugin_catalog_controller.console")
616698
@patch("data_designer.cli.controllers.plugin_catalog_controller.print_error")
617699
def test_run_install_blocks_incompatible_package(
@@ -1068,19 +1150,16 @@ def _plan(
10681150
package_name: str = "data-designer-text-transform",
10691151
requirement: str | None = None,
10701152
source_warning: str | None = None,
1071-
data_designer_protection: str | None = None,
10721153
manager: str = "pip",
10731154
install_mode: str = "pip-environment",
10741155
) -> InstallPlan:
10751156
return InstallPlan(
10761157
package_name=package_name,
1077-
source_description=requirement or package_name,
10781158
command=["python", "-m", "pip", "install", requirement or package_name],
10791159
manager=manager,
10801160
catalog_alias=catalog.alias,
10811161
requirement=requirement,
10821162
source_warning=source_warning,
1083-
data_designer_protection=data_designer_protection,
10841163
data_designer_version="0.5.10",
10851164
install_mode=install_mode,
10861165
)
@@ -1133,3 +1212,46 @@ def _entry(
11331212
},
11341213
}
11351214
)
1215+
1216+
1217+
def _catalog_payload(
1218+
*,
1219+
package_name: str,
1220+
runtime_plugin_name: str,
1221+
install_requirement: str,
1222+
) -> dict[str, object]:
1223+
return {
1224+
"schema_version": 2,
1225+
"packages": [
1226+
{
1227+
"name": package_name,
1228+
"description": "GitHub repository reader",
1229+
"install": {
1230+
"requirement": install_requirement,
1231+
"index_url": "https://nvidia-nemo.github.io/DataDesignerPlugins/simple/",
1232+
},
1233+
"compatibility": {
1234+
"python": {"specifier": ">=3.10"},
1235+
"data_designer": {
1236+
"requirement": "data-designer>=0.5.7",
1237+
"specifier": ">=0.5.7",
1238+
"marker": None,
1239+
},
1240+
},
1241+
"docs": {
1242+
"url": f"https://docs.example.test/plugins/{package_name}/",
1243+
},
1244+
"plugins": [
1245+
{
1246+
"name": runtime_plugin_name,
1247+
"plugin_type": "processor",
1248+
"entry_point": {
1249+
"group": "data_designer.plugins",
1250+
"name": runtime_plugin_name,
1251+
"value": "data_designer_github.plugin:plugin",
1252+
},
1253+
}
1254+
],
1255+
}
1256+
],
1257+
}

0 commit comments

Comments
 (0)