Skip to content

Bigtable: update retry & timeout settings - #5794

Merged
igorbernstein2 merged 4 commits into
googleapis:masterfrom
igorbernstein2:retry-settings
Jul 22, 2019
Merged

Bigtable: update retry & timeout settings#5794
igorbernstein2 merged 4 commits into
googleapis:masterfrom
igorbernstein2:retry-settings

Conversation

@igorbernstein2

Copy link
Copy Markdown
Contributor

Regenerate the client using the updated gapic settings:
googleapis/googleapis@184ab77

And the updated gapic-generator:
googleapis/gapic-generator#2878

Update the javadocs for the new settings

Re-enable the dropRowRange integration test now that the settings are correct

Fixes #<issue_number_goes_here> (it's a good idea to open an issue first for context and/or discussion)

Regenerate the client using the updated gapic settings:
googleapis/googleapis@184ab77
And the updated gapic-generator:
googleapis/gapic-generator#2878
Update the javadocs for the new settings
Re-enable the dropRowRange integration test now that the settings are correct
@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 19, 2019
@codecov

codecovBot commented Jul 19, 2019

Copy link
Copy Markdown

Codecov Report

Merging #5794 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@ Coverage Diff @@## master #5794 +/- ##
============================================
+ Coverage 46.78% 46.79% +0.01% 
Complexity 25648 25648 ============================================
Files 2456 2456 Lines 267507 267593 +86 Branches 30564 30564 ============================================
+ Hits 125142 125229 +87 + Misses 133106 133105 -1 
Partials 9259 9259
Impacted FilesCoverage ΔComplexity Δ
...ud/bigtable/data/v2/stub/BigtableStubSettings.java84.47% <100%> (+2.45%)14 <0> (ø)⬇️
...min/v2/stub/BigtableInstanceAdminStubSettings.java96.13% <100%> (+0.2%)34 <0> (ø)⬇️
...ble/data/v2/stub/EnhancedBigtableStubSettings.java95% <100%> (+0.33%)17 <1> (ø)⬇️
.../admin/v2/stub/BigtableTableAdminStubSettings.java93.31% <100%> (+0.78%)26 <0> (ø)⬇️
...ain/java/com/google/cloud/pubsub/v1/Publisher.java89.89% <0%> (+0.34%)40% <0%> (ø)⬇️

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 415f950...4d804b2. Read the comment docs.

settings =
RetrySettings.newBuilder()
.setInitialRetryDelay(Duration.ofMillis(5L))
.setInitialRetryDelay(Duration.ofMillis(1000L))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Duration.ofSeconds(1) is easier to read

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This code is autogenerated by gapic-generator. So it would have to change in its templates

@igorbernstein2

igorbernstein2 commented Jul 20, 2019

Copy link
Copy Markdown
ContributorAuthor

@elharo thanks for the review, unfortunately I can't address any of the feedback because all of the java is autogenerated and I don't have control over it. The only thing thats handwritten here is javadoc in EnhancedBigtableStubSettings.java and the removal of @Ignore in the test.

PTAL

@elharoelharo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not intimately familiar with GAPIC but it sounds like something much deeper is wrong here. The general principle is that auto-generated code should not be checked into the repo. That's sort of like checking in .class files while the .java files only live on the developer's workstation. Another example: check in the .proto files, not the Java code the proto compiler generates. Only the actual editable source, in whatever form it takes, should be in the repository.

If the source generator is only used to initialize the source code, which is then hand edited as usual, then you can check in the generated code because it is in editable form, but it sounds like that is not the case here.

@igorbernstein2
igorbernstein2 merged commit 74e29b6 into googleapis:masterJul 22, 2019
@igorbernstein2
igorbernstein2 deleted the retry-settings branch July 22, 2019 15:02
benjaminp pushed a commit to benjaminp/google-cloud-java that referenced this pull request Jul 13, 2026
* Bigtable: update retry & timeout settings
Regenerate the client using the updated gapic settings:
googleapis/googleapis@184ab77
And the updated gapic-generator:
googleapis/gapic-generator#2878
Update the javadocs for the new settings
Re-enable the dropRowRange integration test now that the settings are correct
* remove extra file
* Revert "remove extra file"
This reverts commit 6f5d8f3.
* remove extra file
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@igorbernstein2@elharo@kolea2@googlebot