feat(sdk): collapse @distributed + deploy_distributed_managed (fixes one-covenant/basilica-backend#660)#484
Conversation
…eploy_distributed_managed (fixes one-covenant/basilica-backend#660) Collapses three deploy paths into one canonical surface per the SDK API simplification plan (docs/plans/SDK-API-SIMPLIFICATION-PLAN.md on basilica-backend main). Before: users had to pick between - @basilica.distributed(...) (decorator, no mid-run handle) - client.deploy_distributed_managed() (managed-wrapper ceremony) - client.deploy_distributed() (explicit-cleanup factory) Three names, three shapes, same outcome. After: ONE handle (DistributedTraining) is itself context-manager-able. The decorator-call returns it directly; bare call is fire-and-forget, `with train() as training:` opens the context for mid-run orchestration. Implementation - DistributedTraining gains __enter__/__exit__/__aenter__/__aexit__ with best-effort delete on scope exit (swallows delete errors so we never mask a caller exception that's already propagating). - deploy_distributed[_async] and deploy_distributed_managed[_async] emit DeprecationWarning on direct calls. The decorator path passes _emit_deprecation=False so users of the canonical surface see no warning. Methods stay functional for two minor versions. - DistributedTrainingManaged class stays for type-annotation back-compat; the factory that produces it (deploy_distributed_managed) warns. - Version bump 0.29.4 -> 0.29.5 (API-surface change). Test evidence - 14 new tests in test_distributed_canonical_surface.py pin every acceptance criterion from the ticket: __enter__/__exit__ presence, with-block delete contract on normal / exception / delete-failure paths, DeprecationWarning on both legacy factories (sync + async), decorator-internal path stays silent. - 171 SDK tests pass (full suite; pre-existing httpx skip on test_dns_propagation_e2e.py is not in scope). - cargo fmt + cargo clippy basilica-sdk clean. Refs basilica-backend SDK simplification plan ticket SDK-S1.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Collapses three distributed-training deploy paths into ONE canonical surface, per the SDK API simplification plan (
docs/plans/SDK-API-SIMPLIFICATION-PLAN.mdon basilica-backend main).DistributedTrainingbecomes itself context-manager-able (__enter__/__exit__/__aenter__/__aexit__).BasilicaClient.deploy_distributed[_managed][_async]emitDeprecationWarningon direct calls; the@basilica.distributeddecorator path stays silent via an internal_emit_deprecation=Falseflag.test_distributed_canonical_surface.pypins every acceptance criterion.After this PR, users write ONE thing:
Same handle, same semantics, no second factory to learn.
Why
Three names for the same operation taxed every new user. The decorator hid the Training handle (no
.scale()/.benchmid-run), so anyone who wanted orchestration switched todeploy_distributed_managed-- a different name, different shape, same outcome. Collapsing the surface makes the decorator the canonical entry-point and keeps the same auto-cleanup contract whether you writewith train()or call it bare.Cross-repo references
fixeskeyword in the title).docs/plans/SDK-API-SIMPLIFICATION-PLAN.mdon basilica-backend main (SDK-S1 row in the wave map).Implementation
crates/basilica-sdk-python/python/basilica/distributed.pyAdded sync + async context-manager protocol to
DistributedTraining. Best-effortdelete()on scope exit, swallows delete-time exceptions so we never mask a caller-side exception that's already propagating.DistributedTrainingManagedis unchanged (still re-exported for type-annotation back-compat).crates/basilica-sdk-python/python/basilica/__init__.pyAdded
_emit_deprecation: bool = Truetodeploy_distributedanddeploy_distributed_async. Body-levelwarnings.warn(..., DeprecationWarning, stacklevel=2)when set.deploy_distributed_managed[_async]emit their own warning at entry and pass_emit_deprecation=Falseto the underlying deploy call (no double-warn).crates/basilica-sdk-python/python/basilica/decorators.pyDistributedFunction.deploy()passes_emit_deprecation=Falsetoclient.deploy_distributed(...). The user opted into the canonical surface by using the decorator -- they should not see a warning that points at the surface they're already on.crates/basilica-sdk-python/tests/test_distributed_canonical_surface.py14 new tests pinning every ticket acceptance criterion. Pre-fix run: 12/14 fail (proving the anti-pattern exists). Post-fix run: 14/14 pass.
Version bump
crates/basilica-sdk-python/Cargo.toml: 0.29.4 -> 0.29.5crates/basilica-sdk-python/pyproject.toml: 0.29.4 -> 0.29.5Cargo.lock: regenerated viacargo update -p basilica-sdk-pythonCHANGELOG.md:## [0.29.5] - 2026-05-18entry with Added + Deprecated sections.Test plan
test_distributed_canonical_surface.py)test_dns_propagation_e2e.pyis unrelated)cargo fmt --all -- --checkcleancargo clippy -p basilica-sdk --all-targets --all-features -- -D warningsclean.scale()/.delete()/.benchpaths still work). Existing example scripts 20/21/22 continue to function; they will migrate in SDK-S5 (separate ticket).Migration for callers
client.deploy_distributed_managed(...) as t:->@basilica.distributed(...)+with train() as t:client.deploy_distributed(...)+ manual.delete()-> same decorator +withblock (auto-cleanup) OR continue calling and silence the warning (still works for two minor versions).DistributedTrainingManagedkeep working -- the class is unchanged and still re-exported.What is NOT in this PR
command=parameter on@basilica.distributed(SDK-S3, separate ticket).source: Union[str, Path]deprecation in favor of Callable-only (SDK-S4).bench: bool+ lazytraining.bench: BenchResult | Nonecollapse (SDK-S2).