-
Notifications
You must be signed in to change notification settings - Fork 7.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Turn off GCP integrations by default #665
Conversation
🚲 PR staged at http://34.66.119.59 |
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.
Code looks good. I manually tested, following the guidelines, and was able to get the app stood up without errors. The logs posted the following, as the expected default behaviour:
Profiler disabled.
Tracing disabled.
Debugger disabled.
LGTM!
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.
Thanks for letting me review this before merging, @askmeegs!
I've left 2 comments.
🚲 PR staged at http://34.66.119.59 |
🚲 PR staged at http://34.66.119.59 |
1 similar comment
🚲 PR staged at http://34.66.119.59 |
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.
Thanks so much for doing this, @askmeegs!
Changes look good to me!
Ahrar has also done the appropriate testing.
Approved and merged.
Unrelated Note
I realized that the code style (e.g., bracket style) varies across this repository.
But again, this concern is definitely outside the scope of this pull-request.
Thus, I've created a separate GitHub issue about enforcing consistent coding style in JavaScript/Node.js service: #675.
@@ -0,0 +1,25 @@ | |||
# Integrating Online Boutique with Google Cloud |
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.
Praise: Thanks for documenting this, Megan!
* Turn off GCP integrations by default * Add doc * cleanup
Summary: Fixes #689. The `px-online-boutique` demo app attempts to connect to GCP integrations. This causes problems when deployed on non-GCP clusters. This diff turns off GCP integrations by default for the demo app. This change has already been addressed upstream in GCP's repo, but at this time we do not want to update our fork of the repo. Upstream fix: GoogleCloudPlatform/microservices-demo#665 Test Plan: Manually deploy modified yaml file on GCP and EKS cluster. Reviewers: michelle Reviewed By: michelle Signed-off-by: Hannah Troisi <htroisi@pixielabs.ai> Differential Revision: https://phab.corp.pixielabs.ai/D12773 GitOrigin-RevId: cc6645c78a19e33af4ee9bb70b4995ee1125850d
Summary: Fixes pixie-io#689. The `px-online-boutique` demo app attempts to connect to GCP integrations. This causes problems when deployed on non-GCP clusters. This diff turns off GCP integrations by default for the demo app. This change has already been addressed upstream in GCP's repo, but at this time we do not want to update our fork of the repo. Upstream fix: GoogleCloudPlatform/microservices-demo#665 Test Plan: Manually deploy modified yaml file on GCP and EKS cluster. Reviewers: michelle Reviewed By: michelle Signed-off-by: Hannah Troisi <htroisi@pixielabs.ai> Differential Revision: https://phab.corp.pixielabs.ai/D12773 GitOrigin-RevId: cc6645c78a19e33af4ee9bb70b4995ee1125850d
* Turn off GCP integrations by default * Add doc * cleanup
Background
Turns off Google Cloud Monitoring, Tracing, Profiler, and Debugger for all services by default, to re-enable out of the box support for local clusters.
Fixes
#663
Change Summary
DISABLE_PROFILER=1
etc for all services, thus turning off GCP integrations by defaultTesting Procedure
PROJECT_ID
env field set (for GCR)skaffold run --default-repo=gcr.io/$PROJECT_ID/onlineboutique
this will test thekubernetes-manifests
, notrelease/
on a local cluster. that's what you want.