Open
Conversation
Add new crate with the following modules: - error.rs: Comprehensive error hierarchy (BasilicaError, DeploymentError) - volume.rs: Volume type for persistent storage - spec.rs: DeploymentSpec for deployment configuration - model_size.rs: GPU requirements estimation from model names - source.rs: SourcePackager for Python code packaging All tests pass (62 unit tests + 18 doc tests).
- Add basilica-sdk-rust to CI change detection in paths-filter - Add clippy check for basilica-sdk-rust in lint-complex job - Create dedicated test-rust-sdk job with: - Format checking - Clippy linting with -D warnings - Full build - Unit tests via nextest - Doc tests - Add test-rust-sdk to ci-success final status check Clippy fixes: - Use next_back() instead of last() for DoubleEndedIterator - Remove unnecessary Ok wrapping with ? operator - Use div_ceil() instead of manual implementation
📝 WalkthroughWalkthroughIntroduces a comprehensive Rust SDK for Basilica to the workspace, featuring high-level client APIs, deployment management, error handling, GPU requirement estimation, and Python source packaging. Updates CI/CD pipeline to test the new Rust SDK. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@crates/basilica-sdk-rust/src/client.rs`:
- Around line 774-779: The GPU requirements block currently drops the entire
GpuRequirementsSpec when config.gpu_count is None, which can silently ignore
GPU-related fields; update the logic around config.gpu_count and
GpuRequirementsSpec to validate or infer a count: if config.gpu_count.is_none()
but any of config.gpu_models, config.min_cuda_version, or
config.min_gpu_memory_gb is set, either return an error (propagate a Result) or
set a sensible default (e.g., count = 1) before constructing GpuRequirementsSpec
so the GPU constraints are not silently discarded; locate the code building
gpu_spec (the map on config.gpu_count and the GpuRequirementsSpec construction)
and add the validation/inference there.
In `@crates/basilica-sdk-rust/src/deployment.rs`:
- Around line 392-395: In is_http_ready(), remove the insecure
`.danger_accept_invalid_certs(true)` call on the reqwest::Client::builder() used
to construct `client` so TLS certificate validation is enforced; update the
builder chain to omit that method and keep
`.timeout(HTTP_READY_TIMEOUT).build()` and handle the resulting Result from
`build()` as before (propagate or map the error from `client` creation) so
readiness checks validate certificates properly instead of bypassing TLS
verification.
In `@crates/basilica-sdk-rust/src/source.rs`:
- Around line 274-276: The heredoc uses a fixed delimiter "PYCODE" which can
collide with self.code; change the logic in the method that builds the python
command so it generates a unique delimiter (e.g.,
"PYCODE_<random/uuid/timestamp>") and ensure it does not appear in self.code
(loop/regenerate while self.code.contains(&delimiter)). Then use that delimiter
when constructing heredoc (the variable currently named heredoc) before pushing
it into parts; add any small dependency (uuid or rand) or use a timestamp-based
string to create the unique suffix.
In `@crates/basilica-sdk-rust/src/spec.rs`:
- Around line 3-16: The docs claim DeploymentSpec is "immutable" but its public
struct fields allow external mutation; update either the docs or the struct:
make the fields of the DeploymentSpec struct private (change pub fields to
non-pub) and provide read-only accessor methods (e.g., impl DeploymentSpec { pub
fn name(&self) -> &str { &self.name } } or similar for other fields) or
alternatively change the module-level doc comment to remove/soften the
"immutable" wording to reflect that fields are public and mutable; reference the
DeploymentSpec type and its public fields in src/spec.rs when making this change
so callers keep using the new getters or updated docs consistently.
🧹 Nitpick comments (1)
crates/basilica-sdk-rust/src/client.rs (1)
278-283: Avoid:latestimage tags for reproducible deployments.Pin to a known tag or expose the image/tag in
VllmConfig/SglangConfigto avoid unexpected changes.Also applies to: 409-414
Comment on lines
+774
to
+779
| let gpu_spec = config.gpu_count.map(|count| GpuRequirementsSpec { | ||
| count, | ||
| model: config.gpu_models.clone().unwrap_or_default(), | ||
| min_cuda_version: config.min_cuda_version.clone(), | ||
| min_gpu_memory_gb: config.min_gpu_memory_gb, | ||
| }); |
There was a problem hiding this comment.
GPU constraints can be silently ignored when gpu_count is None.
If gpu_models/min_cuda_version/min_gpu_memory_gb are set without gpu_count, the GPU spec is dropped and a CPU deployment results. Consider validating or inferring a default count.
✅ Add validation to avoid silent misconfiguration
+ if config.gpu_count.is_none()
+ && (config.gpu_models.is_some()
+ || config.min_cuda_version.is_some()
+ || config.min_gpu_memory_gb.is_some())
+ {
+ return Err(BasilicaError::Validation {
+ message: "gpu_count is required when specifying GPU constraints.".to_string(),
+ field: Some("gpu_count".to_string()),
+ value: None,
+ });
+ }
+
let gpu_spec = config.gpu_count.map(|count| GpuRequirementsSpec {
count,
model: config.gpu_models.clone().unwrap_or_default(),
min_cuda_version: config.min_cuda_version.clone(),
min_gpu_memory_gb: config.min_gpu_memory_gb,
});📝 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
| let gpu_spec = config.gpu_count.map(|count| GpuRequirementsSpec { | |
| count, | |
| model: config.gpu_models.clone().unwrap_or_default(), | |
| min_cuda_version: config.min_cuda_version.clone(), | |
| min_gpu_memory_gb: config.min_gpu_memory_gb, | |
| }); | |
| if config.gpu_count.is_none() | |
| && (config.gpu_models.is_some() | |
| || config.min_cuda_version.is_some() | |
| || config.min_gpu_memory_gb.is_some()) | |
| { | |
| return Err(BasilicaError::Validation { | |
| message: "gpu_count is required when specifying GPU constraints.".to_string(), | |
| field: Some("gpu_count".to_string()), | |
| value: None, | |
| }); | |
| } | |
| let gpu_spec = config.gpu_count.map(|count| GpuRequirementsSpec { | |
| count, | |
| model: config.gpu_models.clone().unwrap_or_default(), | |
| min_cuda_version: config.min_cuda_version.clone(), | |
| min_gpu_memory_gb: config.min_gpu_memory_gb, | |
| }); |
🤖 Prompt for AI Agents
In `@crates/basilica-sdk-rust/src/client.rs` around lines 774 - 779, The GPU
requirements block currently drops the entire GpuRequirementsSpec when
config.gpu_count is None, which can silently ignore GPU-related fields; update
the logic around config.gpu_count and GpuRequirementsSpec to validate or infer a
count: if config.gpu_count.is_none() but any of config.gpu_models,
config.min_cuda_version, or config.min_gpu_memory_gb is set, either return an
error (propagate a Result) or set a sensible default (e.g., count = 1) before
constructing GpuRequirementsSpec so the GPU constraints are not silently
discarded; locate the code building gpu_spec (the map on config.gpu_count and
the GpuRequirementsSpec construction) and add the validation/inference there.
Comment on lines
+392
to
+395
| let client = reqwest::Client::builder() | ||
| .timeout(HTTP_READY_TIMEOUT) | ||
| .danger_accept_invalid_certs(true) | ||
| .build(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file and context around lines 392-395
wc -l crates/basilica-sdk-rust/src/deployment.rsRepository: PlatformNetwork/basilica
Length of output: 114
🏁 Script executed:
# Read the specific section with context
sed -n '380,410p' crates/basilica-sdk-rust/src/deployment.rsRepository: PlatformNetwork/basilica
Length of output: 952
🏁 Script executed:
# Search for the function containing this code and any related comments
ast-grep --pattern 'let client = reqwest::Client::builder()'Repository: PlatformNetwork/basilica
Length of output: 50
🏁 Script executed:
# Check if there are environment variables or config flags related to TLS
rg -i 'danger_accept_invalid_certs|tls|certificate|https' crates/basilica-sdk-rust/src/deployment.rs -A 2 -B 2Repository: PlatformNetwork/basilica
Length of output: 226
🏁 Script executed:
# Search for where is_http_ready is called
rg 'is_http_ready' crates/basilica-sdk-rust/src/Repository: PlatformNetwork/basilica
Length of output: 250
🏁 Script executed:
# Look for any configuration or environment variables that might control TLS validation
rg -i 'insecure|skip.*cert|tls.*skip' crates/basilica-sdk-rust/ --type rustRepository: PlatformNetwork/basilica
Length of output: 50
🏁 Script executed:
# Check if there's documentation about the security model
find crates/basilica-sdk-rust -name '*.md' -o -name 'README*' | head -20Repository: PlatformNetwork/basilica
Length of output: 50
🏁 Script executed:
# Look at the broader Deployment struct definition
sed -n '1,100p' crates/basilica-sdk-rust/src/deployment.rsRepository: PlatformNetwork/basilica
Length of output: 3377
🏁 Script executed:
# Check if there are other HTTP client usages for comparison
rg 'reqwest::Client' crates/basilica-sdk-rust/src/ -B 2 -A 5Repository: PlatformNetwork/basilica
Length of output: 696
Disable TLS verification bypass in is_http_ready() method.
The .danger_accept_invalid_certs(true) setting leaves the readiness check vulnerable to MITM attacks, where an attacker could forge responses and provide false positive signals that the service is ready. Remove this line to enforce proper TLS validation; self-signed certificates should be handled through proper infrastructure configuration, not by disabling validation in the SDK.
Proposed fix
let client = reqwest::Client::builder()
.timeout(HTTP_READY_TIMEOUT)
- .danger_accept_invalid_certs(true)
.build();📝 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
| let client = reqwest::Client::builder() | |
| .timeout(HTTP_READY_TIMEOUT) | |
| .danger_accept_invalid_certs(true) | |
| .build(); | |
| let client = reqwest::Client::builder() | |
| .timeout(HTTP_READY_TIMEOUT) | |
| .build(); |
🤖 Prompt for AI Agents
In `@crates/basilica-sdk-rust/src/deployment.rs` around lines 392 - 395, In
is_http_ready(), remove the insecure `.danger_accept_invalid_certs(true)` call
on the reqwest::Client::builder() used to construct `client` so TLS certificate
validation is enforced; update the builder chain to omit that method and keep
`.timeout(HTTP_READY_TIMEOUT).build()` and handle the resulting Result from
`build()` as before (propagate or map the error from `client` creation) so
readiness checks validate certificates properly instead of bypassing TLS
verification.
Comment on lines
+274
to
+276
| // Build the python command with heredoc | ||
| let heredoc = format!("python3 - <<'PYCODE'\n{}\nPYCODE\n", self.code); | ||
| parts.push(heredoc); |
There was a problem hiding this comment.
Guard against heredoc delimiter collisions.
If the code contains a line equal to PYCODE, the heredoc will terminate early and truncate execution. Pick a delimiter that doesn't appear in the code.
🔧 Safer heredoc delimiter
- let heredoc = format!("python3 - <<'PYCODE'\n{}\nPYCODE\n", self.code);
+ let mut delimiter = "PYCODE".to_string();
+ while self.code.contains(&delimiter) {
+ delimiter.push('_');
+ }
+ let heredoc = format!(
+ "python3 - <<'{d}'\n{code}\n{d}\n",
+ d = delimiter,
+ code = self.code
+ );🤖 Prompt for AI Agents
In `@crates/basilica-sdk-rust/src/source.rs` around lines 274 - 276, The heredoc
uses a fixed delimiter "PYCODE" which can collide with self.code; change the
logic in the method that builds the python command so it generates a unique
delimiter (e.g., "PYCODE_<random/uuid/timestamp>") and ensure it does not appear
in self.code (loop/regenerate while self.code.contains(&delimiter)). Then use
that delimiter when constructing heredoc (the variable currently named heredoc)
before pushing it into parts; add any small dependency (uuid or rand) or use a
timestamp-based string to create the unique suffix.
Comment on lines
+3
to
+16
| //! This module provides [`DeploymentSpec`], an immutable configuration | ||
| //! for defining how deployments should be created and configured. | ||
|
|
||
| use crate::Volume; | ||
| use basilica_sdk::HealthCheckConfig; | ||
| use serde::{Deserialize, Serialize}; | ||
| use std::collections::HashMap; | ||
|
|
||
| /// Immutable specification for a deployment. | ||
| /// | ||
| /// `DeploymentSpec` defines all the configuration needed to create a deployment | ||
| /// on the Basilica GPU cloud platform. It includes container settings, resource | ||
| /// requirements, networking, and optional GPU specifications. | ||
| /// |
There was a problem hiding this comment.
Docs say immutable but fields are public.
Public fields allow mutation, so the "immutable" wording is misleading. Either adjust the docs or make fields private with setters.
📝 Suggested doc tweak
-//! This module provides [`DeploymentSpec`], an immutable configuration
+//! This module provides [`DeploymentSpec`], a configuration
@@
-/// Immutable specification for a deployment.
+/// Specification for a deployment.📝 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
| //! This module provides [`DeploymentSpec`], an immutable configuration | |
| //! for defining how deployments should be created and configured. | |
| use crate::Volume; | |
| use basilica_sdk::HealthCheckConfig; | |
| use serde::{Deserialize, Serialize}; | |
| use std::collections::HashMap; | |
| /// Immutable specification for a deployment. | |
| /// | |
| /// `DeploymentSpec` defines all the configuration needed to create a deployment | |
| /// on the Basilica GPU cloud platform. It includes container settings, resource | |
| /// requirements, networking, and optional GPU specifications. | |
| /// | |
| //! This module provides [`DeploymentSpec`], a configuration | |
| //! for defining how deployments should be created and configured. | |
| use crate::Volume; | |
| use basilica_sdk::HealthCheckConfig; | |
| use serde::{Deserialize, Serialize}; | |
| use std::collections::HashMap; | |
| /// Specification for a deployment. | |
| /// | |
| /// `DeploymentSpec` defines all the configuration needed to create a deployment | |
| /// on the Basilica GPU cloud platform. It includes container settings, resource | |
| /// requirements, networking, and optional GPU specifications. | |
| /// |
🤖 Prompt for AI Agents
In `@crates/basilica-sdk-rust/src/spec.rs` around lines 3 - 16, The docs claim
DeploymentSpec is "immutable" but its public struct fields allow external
mutation; update either the docs or the struct: make the fields of the
DeploymentSpec struct private (change pub fields to non-pub) and provide
read-only accessor methods (e.g., impl DeploymentSpec { pub fn name(&self) ->
&str { &self.name } } or similar for other fields) or alternatively change the
module-level doc comment to remove/soften the "immutable" wording to reflect
that fields are public and mutable; reference the DeploymentSpec type and its
public fields in src/spec.rs when making this change so callers keep using the
new getters or updated docs consistently.
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
This PR introduces a new high-level Rust SDK crate (
basilica-sdk-rust) that mirrors the functionality of the Python SDK (basilica-sdk-python), providing an ergonomic API for Rust developers.Features
BasilicaClient
deploy()- Deploy applications with source code, images, GPU/CPU resourcesdeploy_vllm()- vLLM inference server deployment templatedeploy_sglang()- SGLang inference server deployment templateget()/list()- Deployment retrievalDeployment Facade
status()- Get current deployment status with phase informationwait_until_ready()- Poll until deployment is ready (with DNS/HTTP verification)logs()- Retrieve deployment logsdelete()- Remove deploymentrefresh()- Update cached stateSupporting Components
DeploymentSpec- Immutable deployment configurationVolume- Persistent storage referencesSourcePackager- Source code packaging with framework detection (FastAPI, Flask, Django)model_sizemodule - GPU requirements estimation from model namesBasilicaErrorhierarchy)CI/CD Integration
Cargo.tomllint-complexjobtest-rust-sdkjob with:cargo fmt)-D warningsci-successfinal status checkTesting
cargo fmtQuick Example
Checklist
basilica-sdk-rustcreated undercrates/directorySummary by CodeRabbit
New Features
Chores