Skip to content

[Bigtable] GCRules.GCRule Javadoc fix for createTable - #5066

Merged
chingor13 merged 4 commits into
googleapis:masterfrom
rahulKQL:gcRule
May 13, 2019
Merged

[Bigtable] GCRules.GCRule Javadoc fix for createTable#5066
chingor13 merged 4 commits into
googleapis:masterfrom
rahulKQL:gcRule

Conversation

@rahulKQL

@rahulKQLrahulKQL commented May 6, 2019

Copy link
Copy Markdown
Contributor

Fixes#4056 and FIxes#4090 (Both of these issues are related to GCRule).

  • Have added a sample code for GCRule in BigtableTableAdminClient's createTable() & createTableAsync().
  • Have added one line note about GCRule execution.

@rahulKQL
rahulKQL requested a review from a teamMay 6, 2019 16:00
@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label May 6, 2019
@rahulKQL

rahulKQL commented May 6, 2019

Copy link
Copy Markdown
ContributorAuthor

@igorbernstein2 Please have a look.

Q: Should we consider to adding this note in client Javadoc as well?

igorbernstein2
igorbernstein2 reviewed May 6, 2019
View reviewed changes
Comment thread...loud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java Outdated
Comment thread...loud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java Outdated
Comment thread...loud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java Outdated
Comment thread...oud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateTableRequest.java Outdated
Comment thread...s/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/models/GCRules.java Outdated
Comment thread...s/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/models/GCRules.java Outdated
@rahulKQL

rahulKQL commented May 6, 2019

Copy link
Copy Markdown
ContributorAuthor

@igorbernstein2
I have addressed feedback comments, Please have a fresh look.

igorbernstein2
igorbernstein2 approved these changes May 6, 2019
View reviewed changes

@igorbernstein2igorbernstein2 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.

LGTM, but please follow up with another PR for equality

Comment thread...oud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/models/CreateTableRequest.java Outdated
@rahulKQLrahulKQL mentioned this pull request May 7, 2019
Merged
@igorbernstein2igorbernstein2 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 7, 2019
@yoshi-kokoroyoshi-kokoro removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels May 7, 2019
@chingor13chingor13 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 7, 2019
@yoshi-kokoroyoshi-kokoro removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels May 7, 2019
@igorbernstein2igorbernstein2 added the api: bigtable Issues related to the Bigtable API. label May 7, 2019
@rahulKQL
GCRules.GCRule sample code on createTable
12250a6
Added sample code on createTable, createTableAsync for `GCRules.GCRule` usage.
Added note for GCRules regarding database compaction
@rahulKQL
Addressing review comments
07ac62a
Updated GCRules note, removed empty params, and updated BigtableTableAdminClient with better example
@codecov

codecovBot commented May 10, 2019•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #5066 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@## master #5066 +/- ##
============================================
+ Coverage 50.4% 50.4% +<.01% 
Complexity 23785 23785 ============================================
Files 2251 2251 Lines 226785 226791 +6 Branches 24966 24966 ============================================
+ Hits 114314 114320 +6 
Misses 103864 103864 Partials 8607 8607
Impacted FilesCoverage ΔComplexity Δ
...google/cloud/bigtable/admin/v2/models/GCRules.java68.86% <ø> (ø)16 <0> (ø)⬇️
...d/bigtable/admin/v2/models/CreateTableRequest.java85.18% <ø> (ø)8 <0> (ø)⬇️
...ud/bigtable/admin/v2/BigtableTableAdminClient.java78.63% <ø> (ø)27 <0> (ø)⬇️
...ud/talent/v4beta1/stub/CompletionStubSettings.java0% <0%> (ø)0% <0%> (ø)⬇️
...acatalog/v1beta1/stub/DataCatalogStubSettings.java0% <0%> (ø)0% <0%> (ø)⬇️
...gle/cloud/bigtable/data/v2/BigtableDataClient.java89.58% <0%> (ø)32% <0%> (ø)⬇️
...le/cloud/pubsub/v1/stub/PublisherStubSettings.java84.06% <0%> (ø)20% <0%> (ø)⬇️
...e/src/main/java/com/google/cloud/storage/Blob.java82.09% <0%> (ø)30% <0%> (ø)⬇️
...ud/talent/v4beta1/stub/JobServiceStubSettings.java72.63% <0%> (+0.14%)18% <0%> (ø)⬇️
...t/v4beta1/stub/ApplicationServiceStubSettings.java80.3% <0%> (+0.15%)15% <0%> (ø)⬇️
... and 4 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 06d6685...fa2afe8. Read the comment docs.

@rahulKQL

rahulKQL commented May 10, 2019

Copy link
Copy Markdown
ContributorAuthor

@sduskis@igorbernstein2
I have rebased this PR with the latest master. Please have a look.

@pmakanipmakani added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 13, 2019
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 13, 2019
@yoshi-automationyoshi-automation added the 🚨 critical P0 critical issue. Requires immediate fix label May 13, 2019
igorbernstein2
igorbernstein2 reviewed May 13, 2019
View reviewed changes
Comment thread...loud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java
Comment thread...loud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java
Comment thread...loud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java Outdated
Comment thread...loud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java
Comment thread...loud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java
Comment thread...loud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java Outdated
igorbernstein2
igorbernstein2 approved these changes May 13, 2019
View reviewed changes

@igorbernstein2igorbernstein2 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.

LGTM. Sorry about the noise, I misread the code

Comment thread...loud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java
Comment thread...loud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java
Comment thread...loud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java
@chingor13chingor13 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 13, 2019
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 13, 2019
@chingor13
chingor13 merged commit 00c8bb1 into googleapis:masterMay 13, 2019
@chingor13chingor13 mentioned this pull request May 15, 2019
Merged
benjaminp pushed a commit to benjaminp/google-cloud-java that referenced this pull request Jul 13, 2026
@rahulKQL@chingor13
[Bigtable] GCRules.GCRule Javadoc fix for createTable (googleapis#5066)
248717b
* GCRules.GCRule sample code on createTable
Added sample code on createTable, createTableAsync for `GCRules.GCRule` usage.
Added note for GCRules regarding database compaction
* Addressing review comments
Updated GCRules note, removed empty params, and updated BigtableTableAdminClient with better example
* Addressing feedback and rearranged gcRule example
* Updated GCRules argument description
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@igorbernstein2igorbernstein2igorbernstein2 approved these changes

Assignees

No one assigned

Labels

api: bigtableIssues related to the Bigtable API.cla: yesThis human has signed the Contributor License Agreement.🚨 criticalP0 critical issue. Requires immediate fix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bigtable: update create table docs to demonstrate GCRules usage Bigtable: document that GCRules are only applied during compaction

7 participants

@rahulKQL@igorbernstein2@chingor13@googlebot@pmakani@yoshi-automation@yoshi-kokoro