feat(sdk): deprecate source: Union[str, Path]; Callable canonical (fixes one-covenant/basilica-backend#663)#487
Conversation
…anonical surface (refs basilica-backend#663)
Collapse the `BasilicaClient.deploy_distributed(source=...)` parameter
to a Callable-only surface via the `@basilica.distributed` decorator.
The `str` and `pathlib.Path` shapes remain functional but now emit a
`DeprecationWarning` pointing at the canonical alternatives.
Implementation:
- `crates/basilica-sdk-python/python/basilica/__init__.py:248-298`:
new `_normalize_source_param(source, emit_deprecation=True)` helper
that warns on `str`/`Path` inputs and stays silent on `Callable`
or when the caller is the decorator (which forwards
`_emit_deprecation=False`, the same gate that already silences
the S1 `deploy_distributed`-itself deprecation).
- `crates/basilica-sdk-python/python/basilica/__init__.py:1340-1346`
(sync) and `:2502-2506` (async): call the helper after the S1
gate but before request building, so warnings emit at the
user-facing boundary with `stacklevel=3`.
Wire contract: untouched. This is a Python-side ergonomics warning
only; the request payload still ships the same `command`/`args`/
`source` shape the operator decodes today. The `Callable` shape is
already what the decorator passes (via `inspect.getsource(...)`); S4
formalises that the decorator IS the canonical input surface.
Tests:
- `crates/basilica-sdk-python/tests/test_source_param_deprecation.py`
(new, 10 tests):
* `str` (inline code) -> warns, message points at the decorator.
* `pathlib.Path` and `str` (file path) -> both warn (same
SourcePackager path).
* `Callable` -> silent (canonical shape; only the S1 warning
which is a separate concern).
* Decorator path -> silent (the
`_emit_deprecation=False` gate covers both S1 and S4).
* S3 factory path (`basilica.distributed(command=[...])`) ->
no source-shape warning.
All async variants covered.
- Pre-fix evidence at
`data/evidence/sdk-simpl-663/01-failing-test-pre-fix.txt`: 6
failures captured the anti-pattern (no warning today).
- Post-fix evidence at
`data/evidence/sdk-simpl-663/02-tests-pass-post-fix.txt`: 211
passed in the full SDK Python suite (e2e test excluded; same
behaviour CI swallows via `|| echo`).
Refs the SDK API simplification plan
(`docs/plans/SDK-API-SIMPLIFICATION-PLAN.md` on basilica-backend
main, Problem 2 / SDK-S4). Sits in the converging surface with
S1 (#484), S2 (#483 / #485) and S3 (#486).
Version bumped 0.29.6 -> 0.29.7 across Cargo.toml, pyproject.toml,
and Cargo.lock to satisfy the basilica-sdk-python version-drift
guard in CI.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent 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)
WalkthroughThis PR implements SDK-S4 deprecation for the ChangesSDK-S4 Source Deprecation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 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 |
Summary
Collapse the
BasilicaClient.deploy_distributed(source=...)parameter to a Callable-only surface via the@basilica.distributeddecorator. Thestrandpathlib.Pathshapes remain functional for the next two minor versions, but now emit aDeprecationWarningpointing at the canonical alternatives.This ships SDK-S4 in the SDK API simplification plan (
docs/plans/SDK-API-SIMPLIFICATION-PLAN.mdonone-covenant/basilica-backend@main, Problem 2 / SDK-S4). Sits in the converging surface alongside the previously-merged S1, S2 and S3 work.What changed
Implementation in
crates/basilica-sdk-python/python/basilica/__init__.py:_normalize_source_param(source, *, emit_deprecation=True)helper (lines 248-298) that:sourceunchanged when it isNone, aCallable, or whenemit_deprecation=False;DeprecationWarning(stacklevel=3) forstrandpathlib.Pathinputs, pointing the user at the@basilica.distributeddecorator and therunpy.run_path('/workspace/...')wrapping pattern for external scripts;basilica-backend#663 / SDK-S4in the warning message itself.deploy_distributed, lines 1340-1346) and the async path (deploy_distributed_async, lines 2502-2506) call the helper immediately after the existing S1_emit_deprecationgate, before request building. The same gate that already silences the S1deploy_distributed-itself deprecation now also silences the S4 source-shape deprecation, so users on the canonical surface (@basilica.distributeddecorator or the S3basilica.distributed(command=[...])factory) see no warnings.Wire contract
Untouched. This is a Python-side ergonomics warning only. The request payload still ships the same
command/args/sourceshape over JSON; the operator's decoding path is unchanged. The Callable shape is already what the decorator passes internally (viainspect.getsource(...)); S4 formalises that the decorator IS the canonical input surface.Tests
New file
crates/basilica-sdk-python/tests/test_source_param_deprecation.py(10 tests, sync + async coverage):TestSourceStringEmitsDeprecation:source='<inline python>'warns; warning message mentions@basilica.distributed.TestSourcePathEmitsDeprecation:source=Path(...)ANDsource='<filepath>'both warn (both flow through the same SourcePackager file-reading path).TestSourceCallableStaysSilent:source=<function>does NOT emit a source-shape warning (only the S1deploy_distributed-itself warning, which is a separate concern handled by SDK-S1).TestDecoratorPathDoesNotEmitSourceDeprecation: both the decorator path (@basilica.distributed->deploy()) and the S3 factory path (basilica.distributed(command=[...], client=...)) emit NO warnings — the_emit_deprecation=Falsegate covers both S1 and S4.Test pattern (
BasilicaClient.__new__+MagicMockstub for the PyO3 binding) mirrorstest_distributed_canonical_surface.pyandtest_distributed_command_factory.py. No network or auth calls fire.Evidence
data/evidence/sdk-simpl-663/01-failing-test-pre-fix.txt): 6 of the new tests fail — captures the anti-pattern (no warning today forstr/Pathsource inputs).data/evidence/sdk-simpl-663/02-tests-pass-post-fix.txt): 211 passed in the full SDK Python suite (the unrelatedtest_dns_propagation_e2e.pyis excluded because it requireshttpx, which is not in the SDK dev deps and not installed in CI either — CI'spytest tests/ -v || echoswallows the collection error).data/evidence/sdk-simpl-663/05-runtime-verify/00-runtime-warnings.txt): captures the actualDeprecationWarningtext the runtime emits to stderr when a user callsdeploy_distributed(source='<str>', ...).Cross-references
docs/plans/SDK-API-SIMPLIFICATION-PLAN.mdonone-covenant/basilica-backend@main(Problem 2 / SDK-S4).deploy_distributeddeprecated in favour of the decorator.benchcollapsed to bool.basilica.distributed(command=[...])factory shape.Version bump
crates/basilica-sdk-pythonbumped0.29.6 -> 0.29.7acrossCargo.toml,pyproject.toml, andCargo.lock. This satisfies the basilica-sdk-python version-drift guard in.github/workflows/ci.yml(Check basilica-sdk-python version consistencystep).Test plan
test_source_param_deprecation.py(10 tests) pass on the worktree.cargo fmt --all -- --check: clean.cargo clippy -p basilica-sdk-python -- -D warnings: no warnings introduced.python -c "import basilica; ..."produces the expectedDeprecationWarningto stderr.Fixes one-covenant/basilica-backend#663
Summary by CodeRabbit
Deprecations
Tests