fix: add region tag, move code around for docs#9952
Conversation
pair with @nicain; this is not typical usage of if __name__ == .. but to make sure documentation and intent of the sample for atlas this should work. Co-authored-by: Nick Cain <nicholascain@google.com>
|
Here is the summary of changes. You are about to add 1 region tag.
You are about to delete 1 region tag.
This comment is generated by snippet-bot.
|
I deleted one too many empty lines.
| @@ -14,15 +14,25 @@ | |||
|
|
|||
| # [START cloudrun_pubsub_server_setup] | |||
There was a problem hiding this comment.
Is there a difference between cloudrun_pubsub_server_setup and cloudrun_pubsub_server?
There was a problem hiding this comment.
not in python, but if I don't have both region tags then audits will say cloudrun_pubsub_server is missing.
There was a problem hiding this comment.
We only need to use one region tag. We don't always have complete 1:1 matches across languages
|
|
||
| if __name__ == "__main__": | ||
| PORT = int(os.getenv("PORT")) if os.getenv("PORT") else 8080 | ||
|
|
||
| # This is used when running locally. Gunicorn is used to run the | ||
| # application on Cloud Run. See entrypoint in Dockerfile. | ||
| app.run(host="127.0.0.1", port=PORT, debug=False) | ||
|
|
There was a problem hiding this comment.
A python expert may weigh in but it may be more idiomatic to have this at the bottom of the file and duplicate the region tags around it
There was a problem hiding this comment.
I didn't encompass this in the description but that's what I was pairing with Nick on before. Proposed solution was the second best (first was creating a separate file).
There was a problem hiding this comment.
The if name section is not being used in the sample. It's being launched by gunicorn in the Dockerfile and in Cloud Run. It would be better to remove it than move it.
remote region tag server_setup
process for changing the name for a region tag is to add the second one, update docs, then remove the old tag. So this requires both region tags currently. Second, remove the if statement. Even if this is not matching the intent of the other samples, this is the preferred method from @engelke
Checklist