feat(sdk)!: 0.30.0 remove deprecated distributed surface (fixes one-covenant/basilica-backend#666)#490
Merged
Conversation
…ovenant/basilica-backend#666) S7 of the SDK API simplification plan. Removes every surface that was deprecated by S1-S4 (basilica-backend issues 660, 661, 662, 663), bumps the SDK to 0.30.0, and routes the canonical @basilica.distributed decorator through private impl methods. The user-facing surface is now exactly ONE shape: the decorator (or the BYO-launcher factory) returns a DistributedTraining context manager. BREAKING CHANGE: removed public symbols - BasilicaClient.deploy_distributed - BasilicaClient.deploy_distributed_async - BasilicaClient.deploy_distributed_managed - BasilicaClient.deploy_distributed_managed_async - DistributedTrainingManaged / DistributedTrainingManagedAsync (classes) - DistributedTraining.wait_until_bench_complete - DistributedTraining.wait_until_bench_complete_async - DistributedTraining.bench_status (property) - BenchStatus re-export from `basilica` (kept as internal type in basilica.distributed for the private _bench_status_raw helper) - source: Union[str, Path] shapes -- only Callable accepted (via decorator) - bench='on-start' / bench='off' string modes -- bench is bool-only - _emit_deprecation kwarg on the (now-private) deploy impl The deploy logic that previously lived behind the public deploy_distributed* methods now lives on the private BasilicaClient._deploy_distributed_impl / _deploy_distributed_impl_async helpers. The decorator (@basilica.distributed) and the BYO-launcher factory (basilica.distributed(command=[...])) both route through these private methods. The post-deploy wait_for_bench polling no longer routes through the removed wait_until_bench_complete wrapper -- it reads training._bench_status_raw directly. The wait_for_bench / bench_timeout kwargs on @basilica.distributed keep their existing semantics (never / best_effort / required). Migration matrix: see CHANGELOG.md [0.30.0] section for the per-symbol mapping (legacy -> canonical). README.md "Migration from the legacy surface" updated to point at the AttributeError / ImportError / ValidationError users will see post-upgrade. PyPI publish is NOT done in this PR (per user-facing release contract; publish is a separate authorized step). Refs the SDK API simplification plan (docs/plans/SDK-API-SIMPLIFICATION-PLAN.md on basilica-backend main). Builds on prior simplification PRs (issue 660 / SDK-S1, issue 661 / SDK-S2, issue 662 / SDK-S3, issue 663 / SDK-S4, issue 664 / SDK-S5 examples migration, issue 665 / SDK-S6 docs rewrite).
Contributor
WalkthroughBasilica Python SDK 0.30.0 removes the public distributed-training entrypoints ( ChangesSDK 0.30.0 Breaking Release: Distributed Training Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/basilica-sdk-python/python/basilica/source.py (1)
473-474: 💤 Low valueRedundant import after module-level change.
textwrapis now imported at module scope (line 30), so this local import is unnecessary.🧹 Remove redundant import
- import textwrap - source = textwrap.dedent(source) + source = textwrap.dedent(source)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/basilica-sdk-python/python/basilica/source.py` around lines 473 - 474, Remove the redundant local import of textwrap inside the function — since textwrap is already imported at module scope (top of the file), delete the line "import textwrap" in the block shown and leave the existing call to textwrap.dedent(source) unchanged (refer to the local import near the source = textwrap.dedent(source) statement to locate the removal).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/basilica-sdk-python/CHANGELOG.md`:
- Line 10: Add a missing link-style reference for the new release header "##
[0.30.0] - 2026-05-18" by appending a `[0.30.0]: <release-URL-or-compare-link>`
entry to the bottom of CHANGELOG.md (matching the pattern used for other
versions), ensuring the reference text exactly matches "0.30.0" so the header
link resolves correctly.
In `@crates/basilica-sdk-python/python/basilica/__init__.py`:
- Around line 2353-2393: The async function uses asyncio.get_event_loop() to
obtain loop/time/sleep; replace that call with asyncio.get_running_loop() to
avoid deprecation and be consistent with other async methods (use the existing
loop variable for loop.time() and asyncio.sleep calls), keeping the rest of the
logic that computes deadline, polls training.refresh_async(), and reads
training._bench_status_raw unchanged.
---
Nitpick comments:
In `@crates/basilica-sdk-python/python/basilica/source.py`:
- Around line 473-474: Remove the redundant local import of textwrap inside the
function — since textwrap is already imported at module scope (top of the file),
delete the line "import textwrap" in the block shown and leave the existing call
to textwrap.dedent(source) unchanged (refer to the local import near the source
= textwrap.dedent(source) statement to locate the removal).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f995aa33-bebe-4187-8b10-d7f5af3bf6c0
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
AGENTS.mdcrates/basilica-cli/src/cli/commands.rscrates/basilica-sdk-python/CHANGELOG.mdcrates/basilica-sdk-python/Cargo.tomlcrates/basilica-sdk-python/README.mdcrates/basilica-sdk-python/pyproject.tomlcrates/basilica-sdk-python/python/basilica/__init__.pycrates/basilica-sdk-python/python/basilica/decorators.pycrates/basilica-sdk-python/python/basilica/distributed.pycrates/basilica-sdk-python/python/basilica/exceptions.pycrates/basilica-sdk-python/python/basilica/source.pycrates/basilica-sdk-python/src/lib.rscrates/basilica-sdk-python/tests/test_bench_bool_simplification.pycrates/basilica-sdk-python/tests/test_bench_status_skipped.pycrates/basilica-sdk-python/tests/test_deploy_distributed_managed.pycrates/basilica-sdk-python/tests/test_distributed.pycrates/basilica-sdk-python/tests/test_distributed_canonical_surface.pycrates/basilica-sdk-python/tests/test_distributed_command_factory.pycrates/basilica-sdk-python/tests/test_source_param_deprecation.py
💤 Files with no reviewable changes (3)
- crates/basilica-sdk-python/tests/test_deploy_distributed_managed.py
- crates/basilica-sdk-python/tests/test_bench_status_skipped.py
- crates/basilica-sdk-python/tests/test_source_param_deprecation.py
|
|
||
| ## [Unreleased] | ||
|
|
||
| ## [0.30.0] - 2026-05-18 |
Contributor
There was a problem hiding this comment.
Add the missing changelog reference link for 0.30.0.
## [0.30.0] is introduced, but there’s no corresponding [0.30.0]: ... reference at the bottom, which breaks link-style navigation consistency for release entries.
Suggested docs patch
+[0.30.0]: https://github.com/one-covenant/basilica/compare/basilica-sdk-python-v0.29.7...basilica-sdk-python-v0.30.0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/basilica-sdk-python/CHANGELOG.md` at line 10, Add a missing link-style
reference for the new release header "## [0.30.0] - 2026-05-18" by appending a
`[0.30.0]: <release-URL-or-compare-link>` entry to the bottom of CHANGELOG.md
(matching the pattern used for other versions), ensuring the reference text
exactly matches "0.30.0" so the header link resolves correctly.
Comment on lines
+2353
to
2393
| """ | ||
| Async variant of :py:meth:`_handle_post_deploy_bench_wait`. Polls | ||
| the operator's bench status directly (does not depend on the | ||
| public ``wait_until_bench_complete_async`` wrapper, which was | ||
| removed in 0.30.0). | ||
| """ | ||
| loop = asyncio.get_event_loop() | ||
| deadline = loop.time() + max(timeout, 0) | ||
| bs = None | ||
| while loop.time() < deadline: | ||
| await training.refresh_async() | ||
| bs = training._bench_status_raw | ||
| if bs is None: | ||
| return | ||
| if bs.is_terminal: | ||
| break | ||
| await asyncio.sleep(min(5, max(timeout // 10, 1))) | ||
| else: | ||
| await training.refresh_async() | ||
| bs = training._bench_status_raw | ||
|
|
||
| if bs is None or not bs.is_terminal: | ||
| msg = ( | ||
| f"wait_for_bench='{mode}': bench did not reach a terminal " | ||
| f"phase within {timeout}s (phase=" | ||
| f"{bs.phase if bs else 'absent'})" | ||
| ) | ||
| if mode == "required": | ||
| raise DistributedError(msg) | ||
| warnings.warn(msg, stacklevel=3) | ||
| return | ||
| if bs is None: | ||
| return | ||
|
|
||
| if bs.phase == BENCH_PHASE_SUCCEEDED: | ||
| return | ||
| msg = ( | ||
| f"bench probe phase={bs.phase}" | ||
| f"{f' message={bs.message!r}' if bs.message else ''}" | ||
| ) | ||
| if mode == "required": | ||
| raise DistributedError( | ||
| f"wait_for_bench='required' but {msg}" | ||
| ) | ||
| raise DistributedError(f"wait_for_bench='required' but {msg}") | ||
| warnings.warn(f"wait_for_bench='best_effort': {msg}", stacklevel=3) |
Contributor
There was a problem hiding this comment.
Inconsistent use of asyncio.get_event_loop() vs asyncio.get_running_loop().
This method uses asyncio.get_event_loop() (lines 2359, 2362) while other async methods in this file correctly use asyncio.get_running_loop() (e.g., line 2222). In Python 3.10+, get_event_loop() inside an already-running async context is deprecated and may emit a DeprecationWarning. Since this is an async method, get_running_loop() is the correct choice.
🔧 Use get_running_loop() for consistency
async def _handle_post_deploy_bench_wait_async(
training: DistributedTraining,
mode: Literal["best_effort", "required"],
timeout: int,
) -> None:
...
- loop = asyncio.get_event_loop()
+ loop = asyncio.get_running_loop()
deadline = loop.time() + max(timeout, 0)
bs = None
- while loop.time() < deadline:
+ while asyncio.get_running_loop().time() < deadline:Alternatively, capture the loop once and reuse:
- loop = asyncio.get_event_loop()
+ loop = asyncio.get_running_loop()
deadline = loop.time() + max(timeout, 0)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| """ | |
| Async variant of :py:meth:`_handle_post_deploy_bench_wait`. Polls | |
| the operator's bench status directly (does not depend on the | |
| public ``wait_until_bench_complete_async`` wrapper, which was | |
| removed in 0.30.0). | |
| """ | |
| loop = asyncio.get_event_loop() | |
| deadline = loop.time() + max(timeout, 0) | |
| bs = None | |
| while loop.time() < deadline: | |
| await training.refresh_async() | |
| bs = training._bench_status_raw | |
| if bs is None: | |
| return | |
| if bs.is_terminal: | |
| break | |
| await asyncio.sleep(min(5, max(timeout // 10, 1))) | |
| else: | |
| await training.refresh_async() | |
| bs = training._bench_status_raw | |
| if bs is None or not bs.is_terminal: | |
| msg = ( | |
| f"wait_for_bench='{mode}': bench did not reach a terminal " | |
| f"phase within {timeout}s (phase=" | |
| f"{bs.phase if bs else 'absent'})" | |
| ) | |
| if mode == "required": | |
| raise DistributedError(msg) | |
| warnings.warn(msg, stacklevel=3) | |
| return | |
| if bs is None: | |
| return | |
| if bs.phase == BENCH_PHASE_SUCCEEDED: | |
| return | |
| msg = ( | |
| f"bench probe phase={bs.phase}" | |
| f"{f' message={bs.message!r}' if bs.message else ''}" | |
| ) | |
| if mode == "required": | |
| raise DistributedError( | |
| f"wait_for_bench='required' but {msg}" | |
| ) | |
| raise DistributedError(f"wait_for_bench='required' but {msg}") | |
| warnings.warn(f"wait_for_bench='best_effort': {msg}", stacklevel=3) | |
| """ | |
| Async variant of :py:meth:`_handle_post_deploy_bench_wait`. Polls | |
| the operator's bench status directly (does not depend on the | |
| public ``wait_until_bench_complete_async`` wrapper, which was | |
| removed in 0.30.0). | |
| """ | |
| loop = asyncio.get_running_loop() | |
| deadline = loop.time() + max(timeout, 0) | |
| bs = None | |
| while asyncio.get_running_loop().time() < deadline: | |
| await training.refresh_async() | |
| bs = training._bench_status_raw | |
| if bs is None: | |
| return | |
| if bs.is_terminal: | |
| break | |
| await asyncio.sleep(min(5, max(timeout // 10, 1))) | |
| else: | |
| await training.refresh_async() | |
| bs = training._bench_status_raw | |
| if bs is None or not bs.is_terminal: | |
| msg = ( | |
| f"wait_for_bench='{mode}': bench did not reach a terminal " | |
| f"phase within {timeout}s (phase=" | |
| f"{bs.phase if bs else 'absent'})" | |
| ) | |
| if mode == "required": | |
| raise DistributedError(msg) | |
| warnings.warn(msg, stacklevel=3) | |
| return | |
| if bs.phase == BENCH_PHASE_SUCCEEDED: | |
| return | |
| msg = ( | |
| f"bench probe phase={bs.phase}" | |
| f"{f' message={bs.message!r}' if bs.message else ''}" | |
| ) | |
| if mode == "required": | |
| raise DistributedError(f"wait_for_bench='required' but {msg}") | |
| warnings.warn(f"wait_for_bench='best_effort': {msg}", stacklevel=3) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/basilica-sdk-python/python/basilica/__init__.py` around lines 2353 -
2393, The async function uses asyncio.get_event_loop() to obtain
loop/time/sleep; replace that call with asyncio.get_running_loop() to avoid
deprecation and be consistent with other async methods (use the existing loop
variable for loop.time() and asyncio.sleep calls), keeping the rest of the logic
that computes deadline, polls training.refresh_async(), and reads
training._bench_status_raw unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
S7 of the SDK API simplification plan. Removes every public surface that
was deprecated by S1-S4 (issues 660, 661, 662, 663 on basilica-backend),
bumps the SDK to 0.30.0, and routes the canonical
@basilica.distributeddecorator through private impl methods on
BasilicaClient. The usercontract is now exactly ONE shape: the decorator (or the BYO-launcher
factory) returns a
DistributedTrainingcontext manager.This is the major-equivalent (pre-1.0) bump:
0.29.7 -> 0.30.0. Theticket allowed
0.30.0OR1.0.0;pyproject.tomlcarriesDevelopment Status :: 4 - Beta, which excludes a1.0.0stabilitydeclaration.
0.30.0is the only target consistent with both"breaking" and "beta".
BREAKING CHANGE
The following public symbols are REMOVED. Calls now raise
AttributeError/ImportError/ValidationError; migrate via themapping below before upgrading.
client.deploy_distributed(source=fn, ...)@basilica.distributed(...)\ndef fn(): ...\ntraining = fn()client.deploy_distributed(source="<inline>", ...)client.deploy_distributed(source=Path("./train.py"), ...)runpy.run_path('/workspace/train.py')inside a decorated functionclient.deploy_distributed_managed(command=[...], ...)basilica.distributed(command=[...], ...)client.deploy_distributed_managed(source=fn, ...)with fn() as training:after@basilica.distributedonfnbench="on-start"bench=Truebench="off"bench=Falsetraining.wait_until_bench_complete(timeout=t)with training:then readtraining.benchtraining.bench_status.phasetraining.bench_diagnostics["phase"]BenchStatus(re-export frombasilica)BenchResult(result payload) or thedictfromtraining.bench_diagnosticsDistributedTrainingManaged/DistributedTrainingManagedAsyncDistributedTraining(itself context-manager-able)Plus internal:
_emit_deprecationkwarg on the (now-private) deployimpl is removed; the deprecation-gating plumbing it controlled no
longer has any deprecation paths to suppress.
Refactor (no public-surface change)
deploy_distributed*methods now lives on the privateBasilicaClient._deploy_distributed_impl/_deploy_distributed_impl_asyncmethods. The decorator(
@basilica.distributed) and the BYO-launcher factory(
basilica.distributed(command=[...])) both route through theseprivate methods.
wait_for_benchpolling no longer routes throughthe removed
wait_until_bench_completewrapper -- it readstraining._bench_status_rawdirectly. Thewait_for_bench/bench_timeoutkwargs on@basilica.distributedkeep theirexisting semantics (
never/best_effort/required).BenchStatusdataclass remains inbasilica.distributedas aninternal type backing
_bench_status_raw(and thereforebench_diagnostics); it is no longer re-exported from the top-levelbasilicapackage.@basilica.distributedis now in ashared helper
basilica.source._package_function_for_torchrun(...);the decorator's
DistributedFunction._extract_source()is a thinshim over the same helper used by the deploy impl, so existing
decorator-introspection tests exercise the production code path.
Files
crates/basilica-sdk-python/python/basilica/__init__.py(removals +private impl),
distributed.py(drop bench_status / wait waiters /Managed wrapper classes),
decorators.py(route through privateimpl),
source.py(shared packager helper),exceptions.py+src/lib.rs(docstring updates).test_bench_bool_simplification.py,test_distributed.py,test_distributed_canonical_surface.py,test_distributed_command_factory.pyupdated to call_deploy_distributed_impl[_async], assert removed-surfaceAttributeError / ValidationError, and read bench data via
training.bench/training.bench_diagnostics. Removed:test_deploy_distributed_managed.py,test_bench_status_skipped.py,test_source_param_deprecation.py(entirely about removed surfaces).
pyproject.toml,Cargo.toml,Cargo.lockbumped to0.30.0. CHANGELOG entry under [0.30.0] documents every removedsymbol + the migration matrix.
crates/basilica-sdk-python/README.md"Migration from thelegacy surface" rewritten to reflect post-removal state.
AGENTS.mdcrates/basilica-cli/src/cli/commands.rsdocstrings updated.Test plan
tests that pinned removed surfaces, which are deleted or
rewritten)
cargo fmt -p basilica-sdk-python -p basilica-cli -- --check:clean
cargo clippy -p basilica-sdk-python --release -- -D warnings:clean
AttributeError/ImportError/ValidationError(evidencein basilica-backend repo's
data/evidence/sdk-simpl-666/02-post-removal-attribute-errors.txt)leaked to the user (evidence dir
05-runtime-verify/)change in
01-pytest-baseline.txt)Not in this PR
delivered).
Cross-refs
docs/plans/SDK-API-SIMPLIFICATION-PLAN.mdonbasilica-backend
main(the SDK-S7 line is the authority for thisscope).
feat(sdk): basilica.distributed(command=...) factory shape (fixes one-covenant/basilica-backend#662) #486 (S3), feat(sdk): deprecate source: Union[str, Path]; Callable canonical (fixes one-covenant/basilica-backend#663) #487 (S4), feat(sdk): migrate examples 20/21/22 to canonical decorator + bench bool + context-manager surface (fixes one-covenant/basilica-backend#664) #488 (S5), docs(sdk): rewrite README and module docstrings around canonical decorator + context-manager surface (fixes one-covenant/basilica-backend#665) #489 (S6).
Summary by CodeRabbit
Release Notes v0.30.0
Breaking Changes
deploy_distributedanddeploy_distributed_managedmethods. Use@basilica.distributeddecorator instead.benchparameter modes. Thebenchoption now accepts boolean values only.BenchStatusaccessor and legacy bench helper methods.Documentation