Skip to content
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

chore(cloudrun): migrate region tags for dockerfiles and yaml from run folder - part 2 - step 3 #13105

Conversation

eapl-gemugami
Copy link
Contributor

Description

Fixes Internal:
b/347349975
b/347349649

Checklist

@product-auto-label product-auto-label bot added samples Issues that are directly related to samples. api: run Issues related to the Cloud Run API. labels Jan 28, 2025
Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

Hi team,

I'm currently reviewing this pull request and will provide a full review shortly. In the meantime, here's a quick summary for you and other reviewers to get up to speed:

This pull request, "chore(cloudrun): migrate region tags for dockerfiles and yaml from run folder - part 2 - step 3", aims to migrate region tags in Dockerfiles and YAML files related to Cloud Run. It addresses internal issues b/347349975 and b/347349649. The changes are focused on removing redundant or obsolete region tags from the following files:

  • run/pubsub/Dockerfile (Lines 16 and 44 removed)
  • run/system-package/Dockerfile (Lines 23 and 27 removed)

The changes primarily involve removing commented-out lines related to region tags, streamlining the Dockerfiles. The author has confirmed that tests and lint checks have passed.

I'll have a more detailed review shortly, including a check for any potential side effects of these changes.


Here's a little haiku to brighten your day:

Tags cleaned, code bright,
Cloud Run now runs smoothly,
Joy fills the review.

Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

This pull request removes region tags for the generic Cloud Run service. It correctly follows the sample guidelines and includes necessary updates to the README.

I noticed that the generic Cloud Run tag cloudrun_pubsub_dockerfile was removed. It's important to confirm that this tag is no longer needed and won't impact any documentation or other dependencies.

@eapl-gemugami eapl-gemugami added the snippet-bot:force-run Force snippet-bot runs its logic label Jan 29, 2025
@snippet-bot snippet-bot bot removed the snippet-bot:force-run Force snippet-bot runs its logic label Jan 29, 2025
Copy link

snippet-bot bot commented Jan 29, 2025

Here is the summary of changes.

You are about to delete 2 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@eapl-gemugami
Copy link
Contributor Author

eapl-gemugami commented Jan 29, 2025

There is a possible violation for removing region tag in use.

I see the documents have been updated by cl/720655627 and confirmed of success in CP.

Checking source code for Page 1 and Page 2, the region has been updated:
Page 1

track-metadata-position="GoogleCloudPlatform/python-docs-samples/run/pubsub/Dockerfile/HEAD/cloudrun_pubsub_dockerfile_python"

Page 2

track-metadata-position="GoogleCloudPlatform/python-docs-samples/run/pubsub/Dockerfile/HEAD/cloudrun_pubsub_dockerfile_python"

@eapl-gemugami
Copy link
Contributor Author

As per discussion with our team, we need to fix the region tag cloudrun_pubsub_dockerfile for other languages and repos, to make the snippet-bot pass.

I'll retry this PR again when migration is finished for Node, Go and .NET.

@eapl-gemugami eapl-gemugami marked this pull request as ready for review February 6, 2025 15:41
@eapl-gemugami eapl-gemugami requested review from a team as code owners February 6, 2025 15:41
@eapl-gemugami
Copy link
Contributor Author

Conflicting region tags for Go, Node and dotnet/csharp repos are fixed now. This PR is ready to be reviewed.

@glasnt glasnt merged commit 15249e1 into GoogleCloudPlatform:main Feb 6, 2025
11 checks passed
@eapl-gemugami eapl-gemugami deleted the paradalicea/chore/cloudrun/fix-region-tags-from-run-dockerfiles-and-yaml-part-2-step-3 branch February 6, 2025 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: run Issues related to the Cloud Run API. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants