-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Read database account properties using CosmosClient and CosmosAsyncClient #45789
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?
Conversation
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.
Pull Request Overview
This PR introduces a new model class, CosmosDatabaseAccount, to expose database account properties and updates various components to support reading database account information.
- Added CosmosDatabaseAccount class exposing key account properties.
- Extended client and internal components (RxDocumentClientImpl, AsyncDocumentClient, CosmosClient, and CosmosAsyncClient) with methods to read the database account using asynchronous APIs.
- Updated ImplementationBridgeHelpers and GlobalEndpointManager to support the new database account functionality.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/models/CosmosDatabaseAccount.java | Introduces a new model class for database account properties. |
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/Utils.java | Adds a utility method to convert an Iterable to a List. |
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RxDocumentClientImpl.java | Adds an asynchronous method to read a CosmosDatabaseAccount and maps service regions. |
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/ImplementationBridgeHelpers.java | Introduces a helper for CosmosDatabaseAccount construction with a slight naming mismatch in region parameters. |
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/GlobalEndpointManager.java | Exposes a getter for the internal DatabaseAccount. |
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/AsyncDocumentClient.java | Adds the readDatabaseAccount() method to the asynchronous client. |
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/CosmosClient.java | Adds a blocking method to obtain the CosmosDatabaseAccount from the async client. |
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/CosmosAsyncClient.java | Adds an asynchronous method to read the CosmosDatabaseAccount. |
Comments suppressed due to low confidence (2)
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RxDocumentClientImpl.java:6244
- Collectors.toUnmodifiableList() is not available in Java 8, which is our baseline. Please replace it with a Java 8 compatible alternative (e.g., using Collectors.collectingAndThen to wrap the List with Collections.unmodifiableList) to maintain compatibility.
= Utils.iterableToList(databaseAccount.getReadableLocations()).stream().map(databaseAccountLocation -> databaseAccountLocation.getName()).collect(Collectors.toUnmodifiableList());
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/ImplementationBridgeHelpers.java:1926
- [nitpick] There is a naming inconsistency: the parameter is named 'writeableRegions' here, while the corresponding field in CosmosDatabaseAccount is 'writeRegions'. Consider aligning the naming (e.g., using 'writeRegions' consistently) to improve clarity.
List<String> writeableRegions,
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
…poseDatabaseAccountProperties # Conflicts: # sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RxDocumentClientImpl.java
…eDatabaseAccountProperties
…all account-level regions.
…all account-level regions.
/azp run java - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
…all account-level regions.
/azp run java - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
…all account-level regions.
…all account-level regions.
…all account-level regions.
/azp run java - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run java - cosmos - kafka |
/azp run java - cosmos - spark |
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
Azure Pipelines successfully started running 1 pipeline(s). |
…poseDatabaseAccountProperties
/** | ||
* Represents the Cosmos DB account. A Cosmos DB account is a container for databases. | ||
* */ | ||
public class CosmosDatabaseAccountResponse { |
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.
Mostly a question at thsi point. From consistency perspective I would have expected a CosmosDatabaseAccountProperties class as well as CosmosDatabaseAccountResponse class (implementing CosmosResponse) - any reason why you diverged form this pattern?
} | ||
|
||
static void initialize() { | ||
ImplementationBridgeHelpers.CosmosDatabaseAccountResponseHelper.setCosmosDatabaseAccountResponseAccessor(CosmosDatabaseAccountResponse::new); |
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.
Please follow the same clone-pattern used elsewhere - otherwise the already high risk that new properties get forgotten in the clone-method gets even higher.
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.
Also pelase use the same comment to separate the accessor form the core logic like used in other classes for conisstency reasons.
* @param shouldUseCache a boolean flag to determine whether to use the CosmosAsyncClient's-internal cache for reading the database account (setting shouldUseFlag to true can return stale data). | ||
* @return the {@link CosmosDatabaseAccountResponse} with the read database account. | ||
*/ | ||
public Mono<CosmosDatabaseAccountResponse> readDatabaseAccount(boolean shouldUseCache) { |
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: I would probably prefer the name to ensure false is the default behavior - so shouldSkipCache or somethig like that
Description
Add public API to read database account properties such as readable / writeable regions, account-level consistency. The
readDatabaseAccount
method takes a boolean flag which determines whether an I/O call to the service is made or theCosmosClient
instance's local cache is used.Usage
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines