Skip to content

[ENH]: (Rust client): fix database resolution #5692

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 21, 2025
Merged

[ENH]: (Rust client): fix database resolution #5692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion rust/api-types/src/user_identity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use serde::{Deserialize, Serialize};
use std::collections::HashSet;

#[derive(Serialize, Deserialize, Debug)]
#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))]
pub struct GetUserIdentityResponse {
pub user_id: String,
pub tenant: String,
pub databases: Vec<String>,
pub databases: HashSet<String>,
Copy link
Contributor

@propel-code-bot propel-code-bot bot Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[BestPractice]

This change from Vec<String> to HashSet<String> is indeed a breaking change for consumers of the api-types crate. While this is a good solution to deduplicate database names, it requires careful handling as a public API change.

According to Rust's semantic versioning guidelines, this requires a major version bump (e.g., 1.0.0 → 2.0.0) since it changes the public API in an incompatible way. Consider:

  1. Adding a migration guide in the changelog
  2. Documenting the behavioral differences (no order guarantee, automatic deduplication)
  3. Potentially providing helper methods for common Vec↔HashSet conversions

This follows RFC 1105 API Evolution guidelines for major breaking changes.

Context for Agents 
[**BestPractice**]

This change from `Vec<String>` to `HashSet<String>` is indeed a breaking change for consumers of the `api-types` crate. While this is a good solution to deduplicate database names, it requires careful handling as a public API change.

According to Rust's semantic versioning guidelines, this requires a major version bump (e.g., 1.0.0 → 2.0.0) since it changes the public API in an incompatible way. Consider:
1. Adding a migration guide in the changelog
2. Documenting the behavioral differences (no order guarantee, automatic deduplication)
3. Potentially providing helper methods for common Vec↔HashSet conversions

This follows RFC 1105 API Evolution guidelines for major breaking changes.

File: rust/api-types/src/user_identity.rs
Line: 9

}
2 changes: 1 addition & 1 deletion rust/chroma/src/client/chroma_http_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ impl ChromaHttpClient {
));
}

let database_name = identity.databases.first().ok_or_else(|| {
let database_name = identity.databases.into_iter().next().ok_or_else(|| {
ChromaClientError::CouldNotResolveDatabaseId(
"Client has access to no databases".to_string(),
)
Expand Down
3 changes: 2 additions & 1 deletion rust/frontend/src/auth/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::collections::HashSet;
use std::fmt::{Display, Formatter};
use std::future::{ready, Future};
use std::pin::Pin;
Expand Down Expand Up @@ -122,7 +123,7 @@ fn default_identity() -> GetUserIdentityResponse {
GetUserIdentityResponse {
user_id: String::new(),
tenant: "default_tenant".to_string(),
databases: vec!["default_database".to_string()],
databases: HashSet::from(["default_database".to_string()]),
}
}

Expand Down
Loading