Skip to content

Conversation

@rahulKQL
Copy link

@rahulKQL rahulKQL commented Sep 20, 2019

This commit contains below changes:

  • Removed deprecated BulkMutationBatcher.java from BigtableDataClient and other reference. As newBulkMutationBatcher(tableId) is stable.

  • Removed deprecated EnhancedBigtableStub#bulkMutateRowsBatchingCallable.

  • Removing deprecated MeasureMutateRowsCallable.java and renamed MeasureMutateRowsCallableV2 to match with this.

  • Renamed V2 from new MutateRowsBatchingDescriptorV2.java.

  • Removed unnecessary qualification from MutateRowsBatchingDescriptor javadoc.

cc: @kolea2 @igorbernstein2

This commit contains below changes:

 - Removed deprecated `BulkMutationBatcher.java` from BigtableDataClient and other reference. As `newBulkMutationBatcher(tableId)` is stable.
 - Removed deprecated `EnhancedBigtableStub#bulkMutateRowsBatchingCallable`.
 - Renamed  V2 from new `MutateRowsBatchingDescriptorV2.java`.
 - Removed unnecessary qualification from `MutateRowsBatchingDescriptor` javadoc.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 20, 2019
Also, removing V2 suffix from the MeasureMutateRowsCallableV2.
Updated test case to adapt with latest class.
Copy link
Contributor

@kolea2 kolea2 left a comment

Choose a reason for hiding this comment

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

Overall LGTM, will let @igorbernstein2 take a look as well. Thanks!


/**
* For internal use only.
* This callable will instrument MutateRows invocations using Opencensus stats.
Copy link
Contributor

Choose a reason for hiding this comment

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

minor - "OpenCensus"

@codecov
Copy link

codecov bot commented Sep 20, 2019

Codecov Report

Merging #6309 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #6309      +/-   ##
============================================
- Coverage     46.86%   46.85%   -0.02%     
+ Complexity    28150    28133      -17     
============================================
  Files          2604     2601       -3     
  Lines        287371   287245     -126     
  Branches      33367    33352      -15     
============================================
- Hits         134688   134585     -103     
+ Misses       142450   142426      -24     
- Partials      10233    10234       +1
Impacted Files Coverage Δ Complexity Δ
...le/cloud/bigtable/data/v2/models/BulkMutation.java 100% <ø> (+2.7%) 10 <0> (+1) ⬆️
...gle/cloud/bigtable/data/v2/BigtableDataClient.java 89.58% <ø> (-0.22%) 32 <0> (-1)
.../stub/mutaterows/MutateRowsBatchingDescriptor.java 100% <100%> (ø) 11 <5> (-3) ⬇️
...ble/data/v2/stub/EnhancedBigtableStubSettings.java 98.73% <100%> (ø) 17 <0> (ø) ⬇️
...ud/bigtable/data/v2/stub/EnhancedBigtableStub.java 98.55% <100%> (+3.09%) 20 <0> (-1) ⬇️
...ta/v2/stub/metrics/MeasuredMutateRowsCallable.java 100% <100%> (ø) 2 <0> (ø) ⬇️
.../stub/mutaterows/MutateRowsUserFacingCallable.java 0% <0%> (-100%) 0% <0%> (-2%)
...igtable/gaxx/retrying/ApiResultRetryAlgorithm.java 15.38% <0%> (-15.39%) 3% <0%> (-3%)
.../v2/stub/mutaterows/MutateRowsAttemptCallable.java 88.11% <0%> (-1%) 18% <0%> (-1%)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28466a2...70ee186. Read the comment docs.

Copy link

@igorbernstein2 igorbernstein2 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for putting this together

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants