Skip to content

Conversation

@igorbernstein2
Copy link
Contributor

Should 20 KB not 20 MB
@igorbernstein2 igorbernstein2 requested a review from a team as a code owner April 27, 2021 19:32
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Apr 27, 2021
@product-auto-label product-auto-label bot added the api: bigtable Issues related to the googleapis/java-bigtable API. label Apr 27, 2021
@igorbernstein2 igorbernstein2 added the type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. label Apr 27, 2021
@igorbernstein2 igorbernstein2 requested a review from mutianf April 27, 2021 19:32
@igorbernstein2 igorbernstein2 added the automerge Merge the pull request once unit tests and other checks pass. label Apr 27, 2021
Copy link
Contributor

@mutianf mutianf left a comment

Choose a reason for hiding this comment

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

LGTM

@gcf-merge-on-green
Copy link

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Apr 28, 2021
Copy link
Contributor

@mutianf mutianf left a comment

Choose a reason for hiding this comment

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

Looks like the test also needs to be updated:

[INFO] Running com.google.cloud.bigtable.data.v2.models.QueryTest Error: Tests run: 13, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.579 s <<< FAILURE! - in com.google.cloud.bigtable.data.v2.models.QueryTest Error: com.google.cloud.bigtable.data.v2.models.QueryTest.filterTestWithExceptions Time elapsed: 0.563 s <<< FAILURE! value of : throwable.getMessage() expected to contain: filter size can't be more than 20MB but was : filter size can't be more than 20KB	at com.google.cloud.bigtable.data.v2.models.QueryTest.filterTestWithExceptions(QueryTest.java:134) Caused by: java.lang.IllegalArgumentException: filter size can't be more than 20KB	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:144)	at com.google.cloud.bigtable.data.v2.models.Query.filter(Query.java:172) 
@igorbernstein2 igorbernstein2 added the automerge Merge the pull request once unit tests and other checks pass. label May 5, 2021
@codecov
Copy link

codecov bot commented May 5, 2021

Codecov Report

Merging #787 (15b5060) into master (f937a18) will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@ Coverage Diff @@ ## master #787 +/- ## ============================================ + Coverage 83.44% 83.49% +0.05%  - Complexity 1311 1315 +4  ============================================ Files 114 114 Lines 7799 7806 +7 Branches 442 446 +4 ============================================ + Hits 6508 6518 +10  + Misses 1036 1033 -3  Partials 255 255 
Impacted Files Coverage Δ Complexity Δ
...om/google/cloud/bigtable/data/v2/models/Query.java 68.80% <100.00%> (ø) 27.00 <0.00> (ø)
...om/google/cloud/bigtable/emulator/v2/Emulator.java 60.65% <0.00%> (+0.81%) 14.00% <0.00%> (ø%)
.../bigtable/admin/v2/models/RestoreTableRequest.java 82.85% <0.00%> (+4.28%) 10.00% <0.00%> (+3.00%)
...ata/v2/stub/metrics/HeaderTracerUnaryCallable.java 100.00% <0.00%> (+10.52%) 3.00% <0.00%> (+1.00%)

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 f937a18...15b5060. Read the comment docs.

@mutianf mutianf added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 5, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 5, 2021
@gcf-merge-on-green
Copy link

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label May 6, 2021
@mutianf mutianf added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 7, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 7, 2021
@mutianf mutianf merged commit 04f8ad4 into master May 7, 2021
@mutianf mutianf deleted the igorbernstein2-patch-1 branch May 7, 2021 14:43
kolea2 pushed a commit to kolea2/java-bigtable that referenced this pull request May 20, 2021
* fix: filter limit constant Should 20 KB not 20 MB * fix test (cherry picked from commit 04f8ad4)
kolea2 added a commit that referenced this pull request May 21, 2021
* fix: filter limit constant Should 20 KB not 20 MB * fix test (cherry picked from commit 04f8ad4) Co-authored-by: Igor Bernstein <igorbernstein@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the googleapis/java-bigtable API. cla: yes This human has signed the Contributor License Agreement. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

5 participants