feat(sdk): basilica.distributed(command=...) factory shape (fixes one-covenant/basilica-backend#662)#486
Conversation
…-covenant/basilica-backend#662) basilica-backend issue 662 / SDK-S3. Collapses the BYO-launcher path into the canonical basilica.distributed surface used for function-body deploys. Today: BYO launcher requires client.deploy_distributed_managed(command=[...]). The _managed suffix signals SDK plumbing rather than user-facing semantics; callers had to learn both the suffixed and unsuffixed shapes and pick between them on cleanup-contract grounds. After: basilica.distributed(command=[...], ...) called WITHOUT decorating a function returns a DistributedTraining directly (factory mode). The same symbol still works as a decorator when given a function. Internally the factory path calls deploy_distributed(_emit_deprecation=False) -- the canonical surface stays silent for both decorator and factory shapes. Pass client= to inject an existing BasilicaClient; otherwise a default one is built lazily (mirrors the decorator's .deploy() semantics). Implementation: - crates/basilica-sdk-python/python/basilica/decorators.py: add command=, args=, client= params on distributed(); when command is set, short-circuit to _deploy_command_factory which builds (or accepts) the client and calls deploy_distributed with _emit_deprecation=False. - crates/basilica-sdk-python/tests/test_distributed_command_factory.py: 8 new tests pinning the factory shape (returns DistributedTraining, is context-manager-able, does not warn, accepts client=, regression- pins the decorator-on-function path, re-pins the managed-BYO DeprecationWarning). - Version bump 0.29.5 -> 0.29.6 (pyproject.toml, Cargo.toml, Cargo.lock). - CHANGELOG entry under [0.29.6]. Refs: SDK API simplification plan (docs/plans/SDK-API-SIMPLIFICATION-PLAN.md on basilica-backend main), Problem 4 / SDK-S3 ticket. Builds on the S1 context-manager wiring (basilica-backend#660 / PR #484) and the S2 bench-bool collapse (basilica-backend#661 / PRs #483, #485).
|
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 (5)
✨ 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 |
1 participant
Summary
SDK-S3 from the SDK API simplification plan. Collapses BYO-launcher deploys
into the canonical
basilica.distributedsurface, dropping the_managedsuffix as the user-facing entry point.
client.deploy_distributed_managed(command=[...])for BYOlauncher. The
_managedsuffix signals SDK plumbing; users must learnboth suffixed/unsuffixed variants and pick on cleanup-contract grounds.
basilica.distributed(command=[...], ...)invoked WITHOUT adecorated function returns a
DistributedTrainingdirectly (factorymode). Same symbol still works as a decorator on a function (decorator
mode unchanged).
with training:opens the canonical context manager(wired in SDK-S1).
Plan + ticket
docs/plans/SDK-API-SIMPLIFICATION-PLAN.mdon basilica-backend main, Problem 4one-covenant/basilica-backend#662one-covenant/basilica#484, merge0860a995) and S2(
one-covenant/basilica#483, mergeca2d2e75;#485test patch mergeb7ce2bdc). Both already landedDistributedTraining.__enter__/__exit__and the_emit_deprecation=Falseplumbing that this S3factory path reuses.
Implementation
crates/basilica-sdk-python/python/basilica/decorators.pycommand: Optional[List[str]],args: Optional[List[str]], andclient: Optional[BasilicaClient]parameters todistributed(...).commandis set, short-circuit to_deploy_command_factory(new helper) which materializes a
BasilicaClientif none wassupplied, calls
deploy_distributed(..., _emit_deprecation=False),and returns the
DistributedTrainingdirectly.command) is byte-for-byte identical to thepre-fix shape; the new branch is additive.
Union[Callable[[Callable], DistributedFunction], DistributedTraining].crates/basilica-sdk-python/tests/test_distributed_command_factory.pydecorators.py:481-650).(
TestDistributedCommandFactory).(
TestDecoratorFormStillWorks,TestManagedBYORedirection,TestFactoryArgumentValidation).0.29.5->0.29.6(pyproject.toml,Cargo.toml,Cargo.lock). CHANGELOG entry under[0.29.6].Failing-then-passing test evidence
Pre-fix (against
mainsource):Post-fix (this branch):
Full SDK suite post-fix (excluding pre-existing
test_dns_propagation_e2e.pywhich needs
httpx): 201 passed, 0 failed.Cross-repo deserialization impact
None. The wire shape stays unchanged --
commandandargsalreadyflow through
deploy_distributed->_build_distributed_request->spec.distributed.command/spec.command. This PR only changes thePython user-facing entry point; no operator / CRD / basilica-backend
deserializer touched. Per
feedback_cross_repo_types.mdaudited.Lint / CI gates
cargo fmt --all -- --checkcleancargo check -p basilica-sdk-pythoncleancargo clippy -p basilica-sdk-python -- -D warningscleanWhat is NOT in this PR
deploy_distributed_managed/deploy_distributed/DistributedTrainingManaged[Async]-- they stay deprecated-but-functionalfor two minor versions per the plan; removed in SDK-S7 at the next major.
examples/21_distributed_torchrun.pyandexamples/22_distributed_with_bench.pyto the factory form -- that isSDK-S5.
source: Union[str, Path]deprecation -- that is SDK-S4.0.29.6-- pending separate user authorization.Test plan
cargo fmt --check/cargo check/cargo clippy -- -D warningsclean--merge(NOT--squash)