-
Notifications
You must be signed in to change notification settings - Fork 0
Description
API Ergonomics Feedback from Production Usage
Context
I've been migrating a Redis monitoring application from raw reqwest calls to the redis-cloud crate. Overall the library design is solid, but I encountered some friction points that could improve developer experience.
Issues & Suggestions
1. Database ID Type Inconsistency
Current:
pub struct Database {
pub database_id: Option<i32>,
// ...
}Issue: The database_id appears to always be present in API responses, but it's typed as Option<i32>. This forces consumers to constantly unwrap:
// Current workaround needed
let dbid = db.database_id.unwrap_or(0);
let id_str = db.database_id.map(|id| id.to_string()).unwrap_or_else(|| "-".to_string());Suggestion:
- If the ID is always present: make it
pub database_id: i32 - If it can genuinely be None: document when/why it would be None
- Consider using a newtype:
pub database_id: DatabaseIdwith helpful methods
2. Critical Fields Hidden in extra JSON
Current:
pub struct Database {
pub database_id: Option<i32>,
pub links: Option<Vec<Link>>,
pub extra: Value, // Contains name, status, provider, privateEndpoint, etc.
}Issue: Essential database fields like name, status, provider, region, privateEndpoint are buried in the extra JSON field. This requires verbose extraction:
// Current workaround - manual extraction
fn get_private_endpoint(db: &Database) -> Option<String> {
db.extra
.get("privateEndpoint")
.and_then(|v| v.as_str())
.map(|s| s.to_string())
}
let name = db.extra.get("name").and_then(|v| v.as_str()).unwrap_or("-");
let status = db.extra.get("status").and_then(|v| v.as_str()).unwrap_or("-");
let provider = db.extra.get("provider").and_then(|v| v.as_str()).unwrap_or("-");Suggestion: Promote common fields to top-level struct members:
pub struct Database {
pub database_id: i32,
pub name: String,
pub status: String,
pub provider: String,
pub region: String,
pub private_endpoint: Option<String>,
pub public_endpoint: Option<String>,
pub redis_version: Option<String>,
// ... other common fields
pub extra: Value, // Keep for truly dynamic/uncommon fields
}Or provide convenience methods:
impl Database {
pub fn name(&self) -> Option<&str> {
self.extra.get("name").and_then(|v| v.as_str())
}
pub fn private_endpoint(&self) -> Option<String> {
self.extra.get("privateEndpoint")
.and_then(|v| v.as_str())
.map(String::from)
}
// etc.
}3. Complex Response Unwrapping for Databases
Current:
// Getting databases requires deep JSON navigation
let handler = DatabaseHandler::new(client);
let response = handler.get_subscription_databases(sub_id, None, None).await?;
// Then extract from nested structure:
let databases = response.extra
.get("subscription")
.and_then(|v| v.as_array())
.and_then(|arr| arr.first())
.and_then(|sub| sub.get("databases"))
.and_then(|v| v.as_array())
.context("Failed to extract databases from response")?;
// Then deserialize each database
let dbs: Vec<Database> = databases.iter()
.filter_map(|db| serde_json::from_value(db.clone()).ok())
.collect();Suggestion: Add helper methods to the response type:
impl GetDatabasesResponse {
pub fn databases(&self) -> Result<Vec<Database>> {
// Handle the extraction internally
}
}
// Or make the handler return the databases directly:
impl DatabaseHandler {
pub async fn get_databases(
&self,
subscription_id: i32,
) -> Result<Vec<Database>> {
// Returns databases directly, not wrapped response
}
}4. Similar Pattern for Subscriptions
Current:
let response = handler.get_account_subscriptions(account_id).await?;
let subscriptions = response.extra
.get("subscriptions")
.and_then(|v| v.as_array())
.context("Failed to extract subscriptions")?;Suggestion: Same as above - either helper methods or return Vec directly.
5. Type Mapping Example
Here's the full mapping code I had to write to convert redis-cloud types to display models:
impl From<redis_cloud::flexible::databases::Database> for DatabaseRow {
fn from(db: redis_cloud::flexible::databases::Database) -> Self {
let get_str = |key: &str| -> String {
db.extra.get(key).and_then(|v| v.as_str()).unwrap_or("-").to_string()
};
let get_f64 = |key: &str| -> Option<f64> {
db.extra.get(key).and_then(|v| v.as_f64())
};
let get_bool = |key: &str| -> bool {
db.extra.get(key).and_then(|v| v.as_bool()).unwrap_or(false)
};
Self {
id: db.database_id.map(|id| id.to_string()).unwrap_or_else(|| "-".to_string()),
name: get_str("name"),
status: get_str("status"),
provider: get_str("provider"),
region: get_str("region"),
version: get_str("redisVersion"),
dataset_size: get_f64("datasetSizeInGb")
.map(|v| format!("{} GB", v))
.unwrap_or_else(|| "0.0 GB".to_string()),
persistence: get_str("dataPersistence"),
replication: get_bool("replication").to_string(),
monitoring: "🔴".into(),
}
}
}This pattern repeats for every entity type - it would be much cleaner if these fields were first-class struct members.
Summary
The library's architecture is solid, but the heavy reliance on extra: Value for core fields creates friction. Consider:
- Type common fields explicitly in structs (name, status, provider, endpoints, etc.)
- Keep
extrafor truly dynamic fields that vary by plan/config - Add convenience methods for response unwrapping
- Document when Optional fields are actually None vs always present
The library is definitely usable (we successfully migrated!), but these changes would significantly improve ergonomics and reduce boilerplate in consuming applications.
Environment
redis-cloudversion: 0.6- Use case: Production monitoring application for Redis Cloud databases