feat: add optional Slurm package - #856
Conversation
Add the shared-namespace leaf package and publish it through the same-version data-designer[slurm] extra. Cover resolver, namespace, base-only isolation, and built-wheel installation behavior.\n\nCloses #852 Signed-off-by: Andre Manoel <amanoel@nvidia.com>
Signed-off-by: Andre Manoel <amanoel@nvidia.com>
Greptile SummaryThe PR adds an independently publishable
|
| Filename | Overview |
|---|---|
| packages/data-designer-slurm/pyproject.toml | Defines the optional Slurm distribution with synchronized dynamic versioning and an exact dependency on the base package. |
| packages/data-designer/pyproject.toml | Adds the exact-version Slurm optional dependency while keeping it absent from base installations. |
| scripts/publish.sh | Includes the Slurm artifact in synchronized releases and publishes it before the package advertising the extra. |
| scripts/test_slurm_package_install.py | Exercises clean built-wheel metadata, installation isolation, dependency resolution, and shared namespace behavior. |
| scripts/audit_package_dependencies.py | Extends dependency auditing to account for explicitly selected static and dynamic extras. |
| uv.lock | Updates the workspace lock to include the new Slurm package and optional dependency relationship. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
User[User installation] --> Base[data-designer]
Base --> Config[data-designer-config]
Base --> Engine[data-designer-engine]
Extra[data-designer slurm extra] --> Base
Extra --> Slurm[data-designer-slurm]
Slurm --> Base
Publish[publish.sh] --> Config
Publish --> Engine
Publish --> Slurm
Publish --> Base
Reviews (5): Last reviewed commit: "docs: generalize Slurm package wording" | Re-trigger Greptile
|
Thanks for putting this together, @andreatnvidia — this is a clean, well-scoped foundation for the Slurm work. SummaryThis PR adds the optional FindingsWarnings — Worth addressing
Suggestions — Take it or leave it
What Looks Good
Structural Impact(graphify, 3.5s) Risk: LOW (localized change)
VerdictNeeds changes — The publish-pipeline gap ( This review was generated by an AI assistant. |
Signed-off-by: Andre Manoel <amanoel@nvidia.com>
|
Thanks for putting this together, @andreatnvidia — the built-wheel coverage gives the new package boundary a strong foundation. SummaryThis PR adds the optional FindingsWarnings — Worth addressing
What Looks Good
VerdictNeeds changes — preserve extra selection per dependency edge and align the scheduled audit/canonical package guidance with the new four-package structure before merge. This review was generated by an AI assistant. |
Signed-off-by: Andre Manoel <amanoel@nvidia.com>
|
@nabinchha Thanks, good catches. Addressed both in
The live audit no longer lists |
Upload the Slurm distribution before data-designer so the exact-version extra is resolvable as soon as the base package is published. Signed-off-by: Andre Manoel <amanoel@nvidia.com>
nabinchha
left a comment
There was a problem hiding this comment.
Thanks for the follow-up, @andreatnvidia — the latest publish-order change is well targeted.
Summary
The new commit publishes data-designer-slurm before data-designer and adds a regression test for that ordering, so the documented data-designer[slurm] install path is resolvable as soon as the base release appears. The implementation still matches #852's packaging-only scope, and I found no new correctness issues.
Findings
No new findings.
The new Greptile comment identifies the symmetric partial-release window: if the Slurm upload succeeds and the base upload fails, direct leaf installs are temporarily unresolvable. That is technically accurate, but the two distributions have exact-version dependencies in both directions, so no sequential upload order can eliminate both failure windows. This order protects the documented extra-based entry point, and #852 explicitly defers complete publication sequencing to the later release workstream.
What Looks Good
- The order now follows the user-facing dependency path: config and engine first, then the Slurm provider, then the base package that advertises the extra.
- The focused regression test clearly preserves that release invariant.
- The dependency-audit fixes from
9290cd12remain sound, and the clean built-wheel installation suite passes on both the PR head and the merged target tree.
Verdict
Ship it — the latest commit addresses the prior publish-order concern without introducing a new blocker.
This review was generated by an AI assistant.
Signed-off-by: Andre Manoel <amanoel@nvidia.com>
📋 Summary
Adds the optional
data-designer-slurmdistribution and exposes it through the exact-versiondata-designer[slurm]extra. This establishes package isolation and shared-namespace behavior before Slurm runtime features are added.🔗 Related Issue
Closes #852
Part of #850
🔄 Changes
data-designer-slurmas a publishable workspace package underdata_designer.slurm.🧪 Testing
.venv/bin/pytest packages/data-designer/tests packages/data-designer-slurm/tests- 1121 passed, 1 skippedmake test-slurm-wheel-install.venv/bin/ruff check --fix ..venv/bin/ruff format ..venv/bin/python scripts/update_license_headers.py --checkuv lock --check✅ Checklist