[ENH]: Database and Tenant Crud enhancements#6119
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
It also consolidates the Spanner tenant and database CRUD flow into a single transactional path with richer validation and error handling, broadens the SysDb error surface, aligns the shared request and response types with chroma_types exports, and refreshes the accompanying tests to cover the new behavior. Affected Areas• This summary was automatically generated by @propel-code-bot |
![propel-code-bot[bot]](https://avatars.githubusercontent.com/in/1179004?s=60&v=4){kind=small}
rust/spanner-migrations/migrations/0003-create_databases_unique_index.spanner.sql
Outdated
Show resolved
Hide resolved
| .map_err(SysDbError::FailedToReadColumn)?; | ||
|
|
||
| // resource_name can be NULL, so we handle the error as None | ||
| let resource_name: Option<String> = row.column_by_name("resource_name").ok(); |
There was a problem hiding this comment.
[Logic] Using .ok() here swallows all errors, including schema mismatches (e.g., if the resource_name column is missing from the query result).
Since resource_name is nullable, requesting it as Option<String> will correctly handle SQL NULLs by returning Ok(None), while still propagating errors for missing columns or type mismatches.
| let resource_name: Option<String> = row.column_by_name("resource_name").ok(); | |
| // resource_name can be NULL, so we handle the error as None | |
| let resource_name: Option<String> = row | |
| .column_by_name("resource_name") | |
| .map_err(SysDbError::FailedToReadColumn)?; |
Context for Agents
Using `.ok()` here swallows all errors, including schema mismatches (e.g., if the `resource_name` column is missing from the query result).
Since `resource_name` is nullable, requesting it as `Option<String>` will correctly handle SQL NULLs by returning `Ok(None)`, while still propagating errors for missing columns or type mismatches.
```suggestion
// resource_name can be NULL, so we handle the error as None
let resource_name: Option<String> = row
.column_by_name("resource_name")
.map_err(SysDbError::FailedToReadColumn)?;
```
File: rust/types/src/tenant.rs
Line: 34
tanujnay112
left a comment
tanujnay112
left a comment
There was a problem hiding this comment.
approved with one comment
2 participants
Description of changes
Summarize the changes made by this PR.
Test plan
How are these changes tested?
Added tests
pytestfor python,yarn testfor js,cargo testfor rustMigration plan
None
Observability plan
None
Documentation Changes
None