KEMBAR78
chore: update Error message for bad bigtable.env variable by sid-dinesh94 · Pull Request #91 · googleapis/java-bigtable · GitHub
Skip to content

Conversation

@sid-dinesh94
Copy link
Contributor

At some point, the allowed settings were changed from "prod" and "emulator" to "cloud" and "emulator".
This change updates the error message associated to the illegal argument exception.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 15, 2019
@igorbernstein2
Copy link
Contributor

Yikes! Sorry about that and thank you for the fix!

However I was wondering if you are modifying that property directly? It's meant to be controlled via a maven profile:
https://github.com/googleapis/java-bigtable/blob/master/google-cloud-bigtable/pom.xml#L260

ie: mvn verify -P bigtable-prod-it

@igorbernstein2 igorbernstein2 changed the title Update Error message for bad bigtable.env variable chore: update Error message for bad bigtable.env variable Nov 15, 2019
@codecov
Copy link

codecov bot commented Nov 15, 2019

Codecov Report

Merging #91 into master will increase coverage by 0.25%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #91      +/-   ##
============================================
+ Coverage     81.07%   81.33%   +0.25%     
  Complexity      937      937              
============================================
  Files            95       95              
  Lines          5849     5849              
  Branches        311      325      +14     
============================================
+ Hits           4742     4757      +15     
  Misses          916      916              
+ Partials        191      176      -15
Impacted Files Coverage Δ Complexity Δ
...om/google/cloud/bigtable/emulator/v2/Emulator.java 60.86% <0%> (+0.86%) 14% <0%> (ø) ⬇️
...able/admin/v2/BaseBigtableInstanceAdminClient.java 62.3% <0%> (+3.14%) 54% <0%> (ø) ⬇️
...igtable/admin/v2/BaseBigtableTableAdminClient.java 60.8% <0%> (+4.02%) 48% <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 b3da95a...55be605. Read the comment docs.

At some point, the allowed settings were changed from "prod" and "emulator" to "cloud" and "emulator".
This change updates the error message associated to the illegal argument exception.
@igorbernstein2
Copy link
Contributor

Hope you don't mind, I amended your commit to comply with conventional commit messages

Copy link
Contributor

@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

@igorbernstein2 igorbernstein2 merged commit 1834fc1 into googleapis:master Nov 15, 2019
@sid-dinesh94
Copy link
Contributor Author

Thanks for the work in checking this in!
We have a need to create the test tables within a VPC Service Controlled security perimeter. This requires us to customize the environment variables.

@igorbernstein2
Copy link
Contributor

igorbernstein2 commented Nov 15, 2019

May I suggest to use a combination?

mvn verify -Pbigtable-prod-it -Dbigtable.project=... -Dbigtable.instance=... -Dbigtable.table=....

The bigtable.env property wasn’t meant to be set directly. Without setting the profile, you will default to the emulator profile which might configure different test cases then the prod profile

@sid-dinesh94
Copy link
Contributor Author

Perfect, thank you for the advice. I will modify the invocation accordingly.

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.

3 participants