-
Notifications
You must be signed in to change notification settings - Fork 144
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
feat: use metadata package #95
base: master
Are you sure you want to change the base?
feat: use metadata package #95
Conversation
Signed-off-by: Julien Breux <[email protected]>
The original idea of this demo container was "no dependencies", and we preferred 20 lines to call metadata server that importing a package for that. That being said, we then added some packages for cloudevents, so I'm fine with this change. |
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.
Zone is different from Region.
As you can see in the original code, we parse the zone to extract the region.
Also, the html template expects a region and talks about region
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 the PR, it's great to simplify to make better use of the available libraries.
@@ -157,7 +115,7 @@ func main() { | |||
Service: service, | |||
Revision: revision, | |||
Project: project, | |||
Region: region, | |||
Zone: zone, |
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.
blocker: Cloud Run uses region not zone. While we can extract the region from the zone, we can't use it directly. We should teach using region and how to extract region from zone.
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, thanks for this relevant comment!
I think region
is missing from metadata
package.
I need to check (sorry for delay) here: https://github.com/googleapis/google-cloud-go/blob/main/compute/metadata/metadata.go
Also note that Cloud Run metadata server now returns the region directly (``/computeMetadata/v1/instance/region`, but I am not totally sure this is supported on GCE. |
Description
Use of the
metadata
package to show users how to properly obtain the metadata of an instance in Cloud Run.Note
Removal of the
region
part in favor of thezone
part.The
region
part is no longer part of the metadata package and API.