Skip to content
This repository has been archived by the owner on Jul 25, 2024. It is now read-only.

Containerize the website #54

Merged
merged 4 commits into from
Jun 10, 2021
Merged

Containerize the website #54

merged 4 commits into from
Jun 10, 2021

Conversation

dinagraves
Copy link
Contributor

@dinagraves dinagraves commented Jun 9, 2021

Adding Dockerfile and gunicorn support.

#29

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jun 9, 2021
@dinagraves dinagraves requested review from ace-n and grayside June 9, 2021 23:06
website/app.py Outdated Show resolved Hide resolved
Co-authored-by: Ace Nassri <anassri@google.com>
Copy link
Contributor

@ace-n ace-n left a comment

Choose a reason for hiding this comment

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

LGTM other than a few nits.

website/Dockerfile Show resolved Hide resolved
website/Dockerfile Show resolved Hide resolved
@ace-n
Copy link
Contributor

ace-n commented Jun 10, 2021

@grayside are you saying that Cloud Run automatically adapts to this value?

My biggest concern here is that environments with multiple CPU cores strikes me as...generic. I'd like to see guidance that is (clearly) specific to Cloud Run here.

@grayside
Copy link
Collaborator

Ace, I'm saying any python container with one CPU should be configured roughly this way, and then based on this and the specific app you adjust the concurrency.

If you want to process more requests per container, you can increase workers inside the container and cloud runs CPU and concurrency outside.

Adding Cloud Run CPU cores documentation link
@dinagraves
Copy link
Contributor Author

@ace-n @grayside It now reads

For environments with multiple CPU cores, increase the number of workers
to be equal to the cores available. To increase CPU cores in Cloud Run, see 
https://cloud.google.com/run/docs/configuring/cpu.

@ace-n
Copy link
Contributor

ace-n commented Jun 10, 2021

@dinagraves that's much better IMHO. @grayside WDYT?

@grayside
Copy link
Collaborator

👍

@dinagraves dinagraves merged commit 70841ac into main Jun 10, 2021
@dinagraves dinagraves deleted the containerize branch June 10, 2021 23:53
grayside pushed a commit that referenced this pull request Jul 22, 2021
* Containerize the website
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants