Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

chore(cloud-sql): update GKE sample comments #10451

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

Merged
merged 4 commits into from
Aug 8, 2023

Conversation

jackwotherspoon
Copy link
Contributor

Adding comments to GKE Cloud SQL sample to highlight recommended use.

@jackwotherspoon jackwotherspoon self-assigned this Jul 24, 2023
@jackwotherspoon jackwotherspoon requested a review from a team July 24, 2023 13:02
@jackwotherspoon jackwotherspoon requested a review from a team as a code owner July 24, 2023 13:02
@product-auto-label product-auto-label bot added api: cloudsql samples Issues that are directly related to samples. labels Jul 24, 2023
Copy link
Collaborator

@dandhlee dandhlee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing for general sample workflow

cloud-sql/mysql/sqlalchemy/deployment.yaml Outdated Show resolved Hide resolved
cloud-sql/mysql/sqlalchemy/deployment.yaml Outdated Show resolved Hide resolved
Comment on lines +78 to +80
# If you are not connecting with Automatic IAM, you can delete
# the following flag.
- "--auto-iam-authn"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just confirming you want this to be on by default? It conflicts with the advice above that using Automatic IAM is recommended but it's on by default here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this being on is fine, enabling it still allows users to connect via regular user and password authn, the flag just enables the additional ability for auto IAM authN and doesn't strictly force it.

cloud-sql/postgres/sqlalchemy/deployment.yaml Outdated Show resolved Hide resolved
@jackwotherspoon jackwotherspoon requested a review from dandhlee July 27, 2023 15:33
@jackwotherspoon
Copy link
Contributor Author

@dandhlee this is ready for another pass when you have a second

Copy link
Collaborator

@dandhlee dandhlee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing for Samples

Copy link
Member

@rsamborski rsamborski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please merge main to this branch to fix Python2.7 testing issue.

@jackwotherspoon jackwotherspoon requested a review from a team as a code owner August 8, 2023 15:53
@jackwotherspoon jackwotherspoon merged commit b5b828a into main Aug 8, 2023
@jackwotherspoon jackwotherspoon deleted the cloudsql-gke-comments branch August 8, 2023 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: cloudsql samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.