feat(sdk): delete_cluster / delete_job in client.rl (issue #558)

[snakescripter9999]

Summary

Adds delete_cluster / delete_job to the SDK's client.rl namespace, completing the RL surface for the 0.34.0 release. The merged namespace (#557) predates basilica-backend's DELETE routes (one-covenant/basilica-backend#1525 — reviewed as a pair with this PR): SDK users could create clusters and jobs but not remove them, falling back to kubectl for cleanup.

Related Issues

Closes #558

Type of Change

• New feature (non-breaking change that adds functionality)

Changes Made

• basilica-sdk: DeleteRlClusterResponse / DeleteRlJobResponse DTOs (camelCase wire shape pinned by a serde test) + delete_rl_cluster / delete_rl_job client methods (name-validated, typed DELETE) — mirroring the existing get_rl_* patterns exactly.
• basilica-sdk-python: rl_delete_cluster / rl_delete_job pymethods (JSON-over-the-boundary, same runtime/py.detach shape as the rest of the binding).
• python client.rl: delete_cluster / delete_job with the semantics documented in the docstrings — delete_cluster is refused with an actionable error while a job is active (the server names the blocking job; deleting the job IS the cancel path); delete_job is valid in any phase (deleting a running job IS cancel).
• The .pyi stub is pyo3_stub_gen-generated and picks the new methods up at build time; no hand edits.

Testing

How Has This Been Tested?

• Unit tests pass (cargo test) — basilica-sdk 93 passed + 7 passed, including the new delete_response_wire_shape pin (deserializes the exact server DTO shapes from one-covenant/basilica-backend#1525).
• Integration tests pass — live verification against staging is queued behind one-covenant/basilica-backend#1525's merge + CD deploy; the first use will be SDK-driven cleanup of the leftover smoke-test clusters (the exact workflow this closes).
• Manual testing completed — cargo clippy clean on both crates; python wrapper compiles.

Test Configuration

• OS: macOS 15 (darwin arm64)
• Rust version: 1.97.1

Checklist

• My code follows the project's style guidelines
• I have run cargo fmt to format my code
• I have run cargo clippy and addressed all warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (docstrings; the CHANGELOG entry ships with the 0.34.0 bump PR)
• My changes generate no new warnings
• Any dependent changes have been merged and published — depends on one-covenant/basilica-backend#1525 (the DELETE routes), green and awaiting review as this PR's pair

Additional Context

Ships in 0.34.0 together with #557 so one release carries the complete beta .rl surface (create/get/wait/delete for clusters and jobs). Release plan per Evan's sign-off: version bump 0.33.0 → 0.34.0, CHANGELOG entry marking client.rl as beta (staging-only).

[covenant-talos]

Walkthrough

This PR completes the RL SDK surface for the 0.34.0 release by introducing delete_cluster and delete_job methods across the Rust client and Python bindings, allowing users to manage cluster and job lifecycles without falling back to kubectl. The Rust SDK gains DeleteRlClusterResponse and DeleteRlJobResponse DTOs along with typed, name-validated delete_rl_cluster and delete_rl_job methods on BasilicaClient. These are surfaced to Python via new PyO3 bindings (rl_delete_cluster/rl_delete_job) and wrapped in the Python client's rl.py module. The semantics enforce that deleting a cluster is refused if a job is active—naming the blocking job to guide the user to delete the job first—while deleting a job is valid in any phase and acts as the cancellation path.

Changes

Cohort / File(s)	Change Summary
Rust SDK Core (crates/basilica-sdk/src/client.rs, crates/basilica-sdk/src/rl.rs)	Adds DeleteRlClusterResponse/DeleteRlJobResponse DTOs with wire-shape tests, and delete_rl_cluster/delete_rl_job methods to BasilicaClient targeting the DELETE routes.
Python PyO3 Bindings (crates/basilica-sdk-python/src/lib.rs)	Exposes rl_delete_cluster and rl_delete_job as pymethods bridging the Python boundary to the Rust client using the standard runtime.detach shape.
Python Client (crates/basilica-sdk-python/python/basilica/rl.py)	Implements delete_cluster and delete_job wrapper methods, including docstrings detailing the active-job refusal semantics and cancel-path behavior.

Estimated review effort: 1/5 (mirrors existing SDK patterns with straightforward wrapper additions and standard DTOs).

Instant overview - a deep technical review follows as a separate comment.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b6dca9c7-6fcc-48c8-95e4-98344ce31175

📥 Commits

Reviewing files that changed from the base of the PR and between fa1ef2a and 259fa46.

📒 Files selected for processing (4)

• crates/basilica-sdk-python/python/basilica/rl.py
• crates/basilica-sdk-python/src/lib.rs
• crates/basilica-sdk/src/client.rs
• crates/basilica-sdk/src/rl.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

Walkthrough

The SDK adds RL cluster and job deletion APIs. Rust methods validate names and send authenticated DELETE requests. Python bindings expose the methods, and Python namespace wrappers return decoded response dictionaries.

Changes

RL deletion support

Layer / File(s)	Summary
Deletion response contracts crates/basilica-sdk/src/rl.rs	Adds public response DTOs for deleted cluster and job names. Tests deserialize both response shapes.
Rust client deletion methods crates/basilica-sdk/src/client.rs	Adds authenticated DELETE methods for RL clusters and jobs. Both methods apply DNS-1035 name validation and return typed responses.
Python deletion adapters crates/basilica-sdk-python/src/lib.rs, crates/basilica-sdk-python/python/basilica/rl.py	Exposes asynchronous Rust deletion methods to Python and adds namespace wrappers that return decoded JSON dictionaries. The docstrings describe active-job deletion rules and job cancellation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 259fa

This PR adds localized SDK deletion methods across the Rust and Python client layers; no actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant PythonRlNamespace
  participant PythonBasilicaClient
  participant RustBasilicaClient
  participant RLAPI
  PythonRlNamespace->>PythonBasilicaClient: delete_cluster(name) or delete_job(name)
  PythonBasilicaClient->>RustBasilicaClient: Invoke asynchronous deletion method
  RustBasilicaClient->>RustBasilicaClient: Validate DNS-1035 name
  RustBasilicaClient->>RLAPI: Send authenticated DELETE request
  RLAPI-->>RustBasilicaClient: Return deletion response
  RustBasilicaClient-->>PythonBasilicaClient: Serialize response as JSON
  PythonBasilicaClient-->>PythonRlNamespace: Return decoded dictionary

Loading

Suggested reviewers: snakescripter999-star, epappas, itzlambda

Poem

A rabbit taps DELETE with care
Two RL names vanish from there
Rust checks each name in line
Python unwraps the JSON sign
Jobs may cancel, clusters wait
The burrow ships deletion state

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 93.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 4 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: adding delete_cluster and delete_job to the RL SDK client. It is concise and specific.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/558-rl-delete-sdk

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[covenant-talos]

PR #559: feat(sdk): delete_cluster / delete_job in client.rl (issue #558)

Summary

This PR completes the RL SDK surface by adding delete operations symmetric with the existing create/get paths: two wire DTOs (DeleteRlClusterResponse, DeleteRlJobResponse) and validated delete_rl_cluster/delete_rl_job methods on the Rust client, the corresponding rl_delete_cluster/rl_delete_job PyO3 bindings using the established py.detach + JSON-over-the-boundary pattern, and Python client.rl façade methods whose docstrings encode the domain semantics (cluster deletion refused while a job is active; deleting a job is the cancel path). The change is purely additive, mirrors existing layering exactly, and pins the deserialization shape with a unit test.

Architecture

sequenceDiagram
    participant Py as client.rl (rl.py)
    participant Core as PyO3 (lib.rs)
    participant SDK as BasilicaClient (client.rs)
    participant API as basilica-backend

    Py->>Core: delete_cluster(name) / delete_job(name)
    Core->>SDK: delete_rl_cluster(&name) / delete_rl_job(&name)
    SDK->>SDK: validate_rl_name(name)
    SDK->>API: DELETE /rl/clusters/{name} or /rl/jobs/{name}
    API-->>SDK: {"name": "..."} or 400 (blocking job named)
    SDK-->>Core: DeleteRl*Response
    Core-->>Py: JSON string → dict

Loading

No structural impact: the new methods follow the identical three-layer pattern already used by rl_get_cluster/rl_get_job (validation in the client, runtime.block_on under py.detach in the bindings, json.loads at the façade).

Issues Found

CRITICAL Issues (Must Fix Before Merge)

None found.

HIGH Severity Issues (Advised to Fix Before Merge)

None found.

MEDIUM Severity Issues (Optional to Fix Before Merge)

1. Sequence the 0.34.0 release so backend DELETE routes are deployed before SDK users receive these methods
   Operability | MEDIUM | Effort: quick win (process)
   • Why: The PR description itself states the endpoints do not exist yet: "Live verification against staging lands once one-covenant/basilica-backend#1525 merges and its CD deploys." The new client code (crates/basilica-sdk/src/client.rs, delete_rl_cluster/delete_rl_job issuing DELETE /rl/clusters/{name} and DELETE /rl/jobs/{name}) will return a routing error against any backend without that PR. If the 0.34.0 SDK ships before the backend CD lands, the advertised docstring semantics ("Refused with an actionable 400…") will instead surface as a generic 404/405, and the promised error naming the blocking job will not materialize.
   • How: Gate the 0.34.0 SDK tag on basilica-backend#1525 being deployed, and record the minimum backend version in the SDK release notes. Run the promised SDK-driven smoke-test-cluster cleanup as the acceptance check before tagging.

LOW Severity Issues (Minor Improvements)

1. Cover the refusal (400) path, not only the happy-path wire shape
   Testing & Docs | LOW | Effort: involved

   • Why: The new test in crates/basilica-sdk/src/rl.rs (delete_response_wire_shape) pins only successful deserialization of {"name":"..."}. The behavior the docstrings promise — delete_cluster "Refused (ValueError) while a job is active — the error names the blocking job" (crates/basilica-sdk-python/python/basilica/rl.py) — depends on the 400 error body surviving map_error_to_python, which has no coverage in this diff.
   • How: Once the backend error envelope is stable (post-basilica-backend#1525), add a mock-server test asserting that the server's blocking-job message reaches the Python ValueError text. Acceptable to land in the follow-up verification PR since the contract is not yet deployed.

2. Populate the PR template
   Testing & Docs | LOW | Effort: quick win

   • Why: The template sections ("Summary", "Changes Made", "Testing", "Checklist") were left blank; the prose at the top is informative but the attestations (cargo fmt, cargo clippy, tests added) are unchecked, which weakens the checklist as a review signal.
   • How: Tick the applicable boxes or delete the template boilerplate in future PRs.

Security Review

Sweep of this diff's actual surface:

• Input validation / path injection: The only user-controlled value is the resource name, and both new client methods call Self::validate_rl_name(name)? before interpolating into the URL path (format!("/rl/clusters/{}", name)), which blocks traversal (../) and path-smuggling into sibling routes — provided validate_rl_name enforces the k8s-name charset (implementation not visible in this diff). This matches the create/get pattern.
• Authz boundaries: DELETE is a destructive operation newly exposed to SDK callers. Authorization (namespace/cluster ownership) must be enforced server-side; the SDK adds no authz logic and reuses the client's existing credential handling. Not verifiable from this diff; flagged for the backend PR's review, not this one.
• Secret handling: No secrets, tokens, or key material touched.
• Injection surfaces: No SQL, shell, template, or deserialization-of-untrusted-data beyond the typed serde DTOs; unknown extra fields are tolerated (safe) and the pinned test guards renames.
• Dependency / supply chain: No new dependencies.
• DoS / resource exhaustion: None introduced; no retries, timeouts, or unbounded loops added.

No security-relevant defect found in this diff.

Suggestions for Improvements

• PEP 257 nit: the multi-line docstrings in rl.py (delete_cluster, delete_job) wrap the summary sentence across lines; making line 1 a self-contained summary ("Delete a cluster.") improves pydoc/help rendering.
• Once the backend contract is verified live, consider noting the minimum backend version in the SDK changelog alongside the new methods.

Positive Observations

• Exact pattern fidelity across all three layers — the new code is indistinguishable in structure from the existing rl_get_* methods, which is the right kind of coherency.
• validate_rl_name is invoked before path construction on both deletes, keeping the validation invariant consistent with the create/get methods.
• The delete_response_wire_shape test pins camelCase deserialization with an explicit rationale comment ("caught here, not by a user") — good defensive contract testing.
• Domain semantics are encoded in docstrings rather than prose elsewhere: "deleting the job IS the cancel path" is exactly the kind of operational fact that belongs at the call site.
• Separate DeleteRlClusterResponse/DeleteRlJobResponse types despite identical shape is deliberate future-proofing, not duplication.
• Commit title feat(sdk): delete_cluster / delete_job in client.rl (issue #558) conforms to Conventional Commits; individual commit messages were not provided for review, so no violations can be flagged.

Recommendation and Next Steps

APPROVE — the change is additive, coherent with existing patterns, input-validated, and wire-pinned by test; the only open item is release sequencing, which the author has already planned for. Next steps: land the staged live verification after basilica-backend#1525 deploys, and backfill error-path test coverage in that follow-up.