Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: filter limit constant #787

Merged
merged 2 commits into from
May 7, 2021
Merged

fix: filter limit constant #787

merged 2 commits into from
May 7, 2021

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
gcf-merge-on-green bot pushed a commit that referenced this pull request May 13, 2021
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>
# for free to join this conversation on GitHub. Already have an account? # 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants