-
Notifications
You must be signed in to change notification settings - Fork 16
docs(samples): the bash scripts for environment setup are added #392
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note that I am still a shadow-reviewer
so you will need an review/approval of another java-samples reviewer on top my review/approval.
@lesv can you review my reviews and the PR as a whole?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you for all the changes. Let's wait for @lesv give an approval on my reviews
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok w/ this if it's really the way you want to go. That said, we really are trying to de-emphasize service accounts during development. It wouldn't be too difficult to create the SA, and add the correct IAM to both the SA and the USER's id and explain who they differer.
It's not considered a good pattern to have SA's on laptops. But if that's the way you want to go...
@@ -99,16 +92,6 @@ To run a code sample from the Cloud Shell, you need to be authenticated using th | |||
export GOOGLE_APPLICATION_CREDENTIALS=~/key.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't we trying to de-emphasize GOOGLE_APPLICATION_CREDENTIALS in favor of gcloud auth application-default login
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using GOOGLE_APPLICATION_CREDENTIALS to request the Retail servicei is a recommented way to do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really? Most of the time we are providing you with credentials.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is discribed in the Retail documentation we are relying creating this tutorials https://cloud.google.com/retail/docs/setting-up#local-environment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That isn't the way things are supposed to be these days.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you reach out to a TechWriter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://cloud.google.com/docs/authentication/production#automatically
From our previous experiences, gcloud CLI often results in using end user credentials instead of the service account credentials in some languages, and causing errors like "Your application has authenticated using end user credentials from the Google Cloud SDK or Google Cloud Shell which are not supported."
In this case, we prefer to have users explicitly set GOOGLE_APPLICATION_CREDENTIALS so that end user credentials won't be used.
service_acc_email="$service_account_id@$project_id.iam.gserviceaccount.com" | ||
gcloud iam service-accounts keys create ~/key.json --iam-account "$service_acc_email" | ||
|
||
# activate the service account using the key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, if your doing this, then why tell the user to provide a service account in the README?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the tutorials we give the user 2 options - eanther users want to setup the work env by themselfs, then we provide steps similar to those discribed in the README, or they want to speedup the process and start exploring the Retail features - for tha purpose we give them this scripts.
But you are right, it is good to tell users that they may use the scripts in the Readme as well
# limitations under the License. | ||
|
||
# set the key as GOOGLE_APPLICATION_CREDENTIALS | ||
export GOOGLE_APPLICATION_CREDENTIALS=~/key.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You do know that this will not be set once the script ends?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that, but we still need this variable to run the java code samples within the script
P.S. if you are waiting on me for something, feel free to DM me. I don't always see the messages. |
Sigh - Just realized that what I should have said is we can add the retail api to the allowed permissions in j-d-s-t for testing. That would eliminate the need for your scripts during testing. |
This PR is ready to be merged. I disagree with your use of the service account in development, but I'm not going to block you on it. I had OwlBot run, so you might wish to make sure that everything you want to say you say. - OwlBot sometimes changes things unexpectedly. |
@@ -99,16 +92,6 @@ To run a code sample from the Cloud Shell, you need to be authenticated using th | |||
export GOOGLE_APPLICATION_CREDENTIALS=~/key.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://cloud.google.com/docs/authentication/production#automatically
From our previous experiences, gcloud CLI often results in using end user credentials instead of the service account credentials in some languages, and causing errors like "Your application has authenticated using end user credentials from the Google Cloud SDK or Google Cloud Shell which are not supported."
In this case, we prefer to have users explicitly set GOOGLE_APPLICATION_CREDENTIALS so that end user credentials won't be used.
🤖 I have created a release *beep* *boop* --- ### [2.0.19](v2.0.18...v2.0.19) (2022-04-19) ### Documentation * **samples:** the bash scripts for environment setup are added ([#392](#392)) ([a20b6fb](a20b6fb)) ### Dependencies * update dependency com.google.cloud:google-cloud-bigquery to v2.10.10 ([#407](#407)) ([ad0f7ad](ad0f7ad)) * update dependency com.google.cloud:google-cloud-bigquery to v2.10.7 ([#402](#402)) ([89e94f5](89e94f5)) * update dependency com.google.cloud:google-cloud-bigquery to v2.10.8 ([#403](#403)) ([12f61a1](12f61a1)) * update dependency com.google.cloud:google-cloud-bigquery to v2.10.9 ([#405](#405)) ([6506ebc](6506ebc)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v2.10.0 ([#404](#404)) ([269629d](269629d)) * update dependency com.google.cloud:google-cloud-storage to v2.6.1 ([#406](#406)) ([d35579c](d35579c)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> ☕️
If you write sample code, please follow the samples format.