-
Notifications
You must be signed in to change notification settings - Fork 275
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
Use async cosmos library and upgrade to 2.6.3 #1321
Conversation
ambry-cloud/src/main/java/com.github.ambry.cloud/azure/CosmosDataAccessor.java
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #1321 +/- ##
=========================================
Coverage ? 72.16%
Complexity ? 6498
=========================================
Files ? 472
Lines ? 37265
Branches ? 4707
=========================================
Hits ? 26893
Misses ? 9120
Partials ? 1252
Continue to review full report at Codecov.
|
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.
Generally looks good. A few questions inline.
ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java
Outdated
Show resolved
Hide resolved
ambry-cloud/src/test/java/com.github.ambry.cloud/azure/AzureCloudDestinationTest.java
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java
Outdated
Show resolved
Hide resolved
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.
LGTM. Approved after addressing one remaining comment.
ambry-cloud/src/main/java/com.github.ambry.cloud/azure/AzureCloudDestination.java
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/azure/CosmosDataAccessor.java
Outdated
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/azure/CosmosDataAccessor.java
Outdated
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/azure/CosmosDataAccessor.java
Outdated
Show resolved
Hide resolved
ambry-cloud/src/main/java/com.github.ambry.cloud/azure/CosmosDataAccessor.java
Outdated
Show resolved
Hide resolved
3108e13
to
f72d240
Compare
* @param documentList {@link List <Document>} of documents to return from mocked call. | ||
* @param mockResponse {@link Observable} mocked response. | ||
*/ | ||
static void mockObservableForQuery(List<Document> documentList, Observable<FeedResponse<Document>> mockResponse) { |
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.
Minor: this method can create and return the mockResponse like the others. Okay to change in follow up PR.
This changes replaces the cosmos documentdb library with async cosmosdb library.