-
Notifications
You must be signed in to change notification settings - Fork 363
Add container client options #4992
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
4a1f387
1fbce67
8863124
5bc87aa
2dbe142
737fe5d
d48781d
b9df6fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,13 +49,14 @@ impl ContainerClient { | |
| context: ClientContext, | ||
| database: &ResourceIdentity, | ||
| container: ResourceIdentity, | ||
| options: crate::options::ContainerClientOptions, | ||
| ) -> crate::Result<Self> { | ||
| // The container's addressing mode must match the database's: name-with-name | ||
| // or RID-with-RID. Mixing the two is not supported by the service routing. | ||
| let container_ref = match (database, &container) { | ||
| (ResourceIdentity::Name(db_name), ResourceIdentity::Name(container_name)) => context | ||
| .driver | ||
| .resolve_container(db_name, container_name) | ||
| .resolve_container(db_name, container_name, options.operation) | ||
| .await | ||
| .map_err(|e| { | ||
| azure_data_cosmos_driver::error::CosmosErrorBuilder::from_error(e) | ||
|
|
@@ -77,38 +78,20 @@ impl ContainerClient { | |
| )) | ||
| .build() | ||
| })?; | ||
|
|
||
| // The parent database RID is derived from the container RID, not | ||
| // taken from this `DatabaseClient`. Reject a container whose parent | ||
| // database does not match the addressed database so callers can't | ||
| // accidentally reach into a different database. | ||
| if resolved.database_rid() != db_rid.as_str() { | ||
| return Err(azure_data_cosmos_driver::error::CosmosError::builder() | ||
| .with_status( | ||
| azure_data_cosmos_driver::error::CosmosStatus::CLIENT_INVALID_RESOURCE_ID, | ||
| ) | ||
| .with_message(format!( | ||
| "container RID '{}' belongs to database '{}', not the addressed database '{}'", | ||
| container_rid.as_str(), | ||
| resolved.database_rid(), | ||
| db_rid.as_str() | ||
| )) | ||
| .with_status(azure_data_cosmos_driver::error::CosmosStatus::CLIENT_INVALID_RESOURCE_ID) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: I seem to see a bunch of reformatting from one line to multiple and back, is there something we can do to reduce noise here (like configuring a longer max line length) or something? |
||
| .with_message(format!("container RID '{}' belongs to database '{}', not the addressed database '{}'", container_rid.as_str(), resolved.database_rid(), db_rid.as_str())) | ||
| .build() | ||
| .into()); | ||
| } | ||
|
|
||
| resolved | ||
| } | ||
| (ResourceIdentity::Name(_), ResourceIdentity::Rid(_)) | ||
| | (ResourceIdentity::Rid(_), ResourceIdentity::Name(_)) => { | ||
| return Err(azure_data_cosmos_driver::error::CosmosError::builder() | ||
| .with_status( | ||
| azure_data_cosmos_driver::error::CosmosStatus::CLIENT_MIXED_NAME_RID_ADDRESSING, | ||
| ) | ||
| .with_message( | ||
| "database and container must use the same addressing mode: \ | ||
| address both by name or both by RID", | ||
| ) | ||
| .with_status(azure_data_cosmos_driver::error::CosmosStatus::CLIENT_MIXED_NAME_RID_ADDRESSING) | ||
| .with_message("database and container must use the same addressing mode: address both by name or both by RID") | ||
| .build() | ||
| .into()); | ||
| } | ||
|
|
@@ -119,7 +102,6 @@ impl ContainerClient { | |
| context, | ||
| }) | ||
| } | ||
|
|
||
| /// Builds the SDK-side [`CosmosOperationContext`] for this container's | ||
| /// operations, carrying the operation name plus the database and container | ||
| /// identity the driver context does not know. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| // Licensed under the MIT License. | ||
|
|
||
| use crate::clients::{ClientContext, ContainerClient}; | ||
| use crate::options::ContainerClientOptions; | ||
| use crate::{ResourceId, ResourceIdentity}; | ||
| #[cfg(feature = "control_plane")] | ||
| use azure_data_cosmos_driver::models::DatabaseReference; | ||
|
|
@@ -71,28 +72,24 @@ impl DatabaseClient { | |
| /// Gets a [`ContainerClient`] that can be used to access the container with the | ||
| /// specified identity. | ||
| /// | ||
| /// This method eagerly resolves immutable container metadata (resource ID and partition key | ||
| /// definition) from the service, so the returned client is ready for immediate use without | ||
| /// per-operation cache lookups. | ||
| /// | ||
| /// The container's addressing mode must match this database's: a name-addressed | ||
| /// database accepts only name-addressed containers, and a RID-addressed database | ||
| /// accepts only [`ResourceId`](crate::ResourceId)-addressed containers. | ||
| /// This method eagerly resolves immutable container metadata before returning the client. | ||
| /// | ||
| /// # Arguments | ||
| /// * `container` - The name or RID of the container. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns an error if the container does not exist, the metadata cannot be | ||
| /// resolved, or the addressing mode does not match this database's. | ||
| /// * `options` - Optional parameters for creating the client. | ||
|
Comment on lines
-74
to
+79
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a mis-merge, or did you remove the other content on purpose? |
||
| pub async fn container_client( | ||
| &self, | ||
| container: impl Into<ResourceIdentity>, | ||
| options: Option<ContainerClientOptions>, | ||
| ) -> crate::Result<ContainerClient> { | ||
|
analogrelay marked this conversation as resolved.
|
||
| ContainerClient::new(self.context.clone(), &self.identity, container.into()).await | ||
| ContainerClient::new( | ||
| self.context.clone(), | ||
| &self.identity, | ||
| container.into(), | ||
| options.unwrap_or_default(), | ||
| ) | ||
| .await | ||
| } | ||
|
|
||
| /// Returns the identity (name or RID) used to construct this client. | ||
| pub fn id(&self) -> &ResourceIdentity { | ||
| &self.identity | ||
|
|
@@ -404,6 +401,7 @@ mod tests { | |
| fn _assert_futures_are_send() { | ||
| fn assert_send<T: Send>(_: T) {} | ||
| let client: &DatabaseClient = todo!(); | ||
| assert_send(client.container_client(todo!(), None)); | ||
| let container_identity: ResourceIdentity = todo!(); | ||
| assert_send(client.container_client(container_identity)); | ||
| assert_send(client.read(todo!())); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -97,6 +97,7 @@ pub async fn container_crud_simple() -> Result<(), Box<dyn Error>> { | |
| } | ||
| assert_eq!(vec![properties.id.clone()], ids); | ||
|
|
||
| let container_client = db_client.container_client(&properties.id, None).await?; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this just a duplicate line from the one below? |
||
| let container_client = db_client.container_client(properties.id.as_ref()).await?; | ||
| let mut updated_indexing_policy = IndexingPolicy::default(); | ||
| updated_indexing_policy.automatic = false; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could you link the pr