Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

docs(samples): the bash scripts for environment setup are added #392

Merged
merged 10 commits into from Apr 7, 2022

Conversation

t-karasova
Copy link
Contributor

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:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> ☕️

If you write sample code, please follow the samples format.

@t-karasova t-karasova requested a review from a team as a code owner March 31, 2022 15:03
@product-auto-label product-auto-label bot added api: retail Issues related to the googleapis/java-retail API. samples Issues that are directly related to samples. labels Mar 31, 2022
Copy link
Member

@Shabirmean Shabirmean left a 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?

samples/interactive-tutorials/user_environment_setup.sh Outdated Show resolved Hide resolved
samples/interactive-tutorials/user_environment_setup.sh Outdated Show resolved Hide resolved
samples/interactive-tutorials/user_environment_setup.sh Outdated Show resolved Hide resolved
samples/interactive-tutorials/user_environment_setup.sh Outdated Show resolved Hide resolved
samples/interactive-tutorials/user_environment_setup.sh Outdated Show resolved Hide resolved
samples/interactive-tutorials/user_environment_setup.sh Outdated Show resolved Hide resolved
samples/interactive-tutorials/user_environment_setup.sh Outdated Show resolved Hide resolved
samples/interactive-tutorials/user_environment_setup.sh Outdated Show resolved Hide resolved
@Shabirmean Shabirmean requested a review from lesv April 1, 2022 15:27
@kweinmeister kweinmeister added the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 1, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 1, 2022
@kweinmeister kweinmeister added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Apr 1, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 1, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 1, 2022
Copy link
Member

@Shabirmean Shabirmean left a 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

Copy link

@lesv lesv left a 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
Copy link

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 ?

Copy link
Contributor Author

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.

Copy link

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.

Copy link
Contributor Author

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

Copy link

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.

Copy link

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?

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
Copy link

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?

Copy link
Contributor Author

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
Copy link

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?

Copy link
Contributor Author

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

@lesv
Copy link

lesv commented Apr 4, 2022

P.S. if you are waiting on me for something, feel free to DM me. I don't always see the messages.

@lesv
Copy link

lesv commented Apr 5, 2022

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.

@lesv lesv added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:ignore instruct owl-bot to ignore a PR labels Apr 5, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 6, 2022
@kweinmeister kweinmeister added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 6, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 6, 2022
@lesv lesv added owlbot:run Add this label to trigger the Owlbot post processor. and removed owlbot:ignore instruct owl-bot to ignore a PR labels Apr 6, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 6, 2022
@lesv
Copy link

lesv commented Apr 6, 2022

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

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.

@kweinmeister kweinmeister merged commit a20b6fb into googleapis:main Apr 7, 2022
gcf-merge-on-green bot pushed a commit that referenced this pull request Apr 19, 2022
🤖 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).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api: retail Issues related to the googleapis/java-retail API. samples Issues that are directly related to samples.
Projects
None yet
8 participants