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

Move from docker-build-deploy to python-deploy #215

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

mccalluc
Copy link
Contributor

I had initially planned to leave everything in place, and just add a requirements.txt, and a new workflow, but it looks like if a Dockerfile is present it does a Docker build: the deploy-cloudrun workflow wraps gcloud run deploy which explains:

If a Dockerfile is present in the source code directory, the uploaded source code is built using that Dockerfile. If no Dockerfile is present in the source code directory, Google Cloud's buildpacks automatically detects the language you are using and fetches the dependencies of the code to make a production-ready container image, using a secure base image managed by Google.

... so I instead tried to make this PR clean, so if it seems to work for you, it could be merged as-is:

  • All references to pipenv and Pipfile replaced in docs and workflows.
  • Instead of adding a new test workflow, modified deployment.yml in place.
  • Instead of setting up gcloud CLI, do everything with CI actions.

If you want to go forward with this, I'd suggest:

  • Do not merge this PR! Instead make a new TEST-main branch, and merge this branch to that.
  • Fix the project_id: I've set it to TEST-clima-316917 to make sure the live site isn't clobbered.
  • Confirm that you have GCP_CREDENTIALS in place: This is what the example used, but there are probably many ways to do this.
  • Confirm that the user has the necessary privs.

If all of that seems good, make a commit with bump version to TEST-main and see what happens.

This is a big change, and generally: "if it ain't broke, don't fix it" -- so no offense if you don't go forward with this!

@FedericoTartarini
Copy link
Contributor

Thank you very much for working on this, I am always in favour of testing new things even if the old ones are still working since we should try to improve.

I have a few minor comments/comments:

  • personally I like to have a Pipfile. Can we keep it and then generate the requirements.txt as I was doing before?
  • have you had a chance if changing how we deploy the application is beneficial in terms of performance?

I am quite busy this week, so I may only be able to test this next week.

@mccalluc
Copy link
Contributor Author

mccalluc commented Dec 1, 2023

No rush on this!

personally I like to have a Pipfile. Can we keep it and then generate the requirements.txt as I was doing before?

yes... but it would need to be kept in sync, and I'm not sure about the best way of ensuring that: I'll git it some thought.

The Pipfile is nice, but I wonder if there are particular bits that are important to you that we might be able to provide another way? Off-hand, some of the nice features it has include:

  • Pinning python version and not just dependencies
  • Adding dependencies from the commandline instead of editing a file
  • Prod and dev dependencies in one file
  • Hashes on dependencies in the lock file

(It is a nice tool; It's just annoying that it's not supported directly by cloud run from source.)

have you had a chance if changing how we deploy the application is beneficial in terms of performance?

The development server is single-threaded, which would be a bottleneck with enough traffic. For example if the EPW load takes a while to return, other requests would be blocked. Gcloud does have autoscaling, so maybe that has been kicking in, which would ameliorate the performance, but the guidance is uniform that the flask dev server should not be used for a production site.

@FedericoTartarini
Copy link
Contributor

I tried to implement all the changes you recommended to deploy the app from source but I am facing a major problem. I pushed the code to the repository called development for you to review.

I was able to deploy this code from the source using these commands:

gcloud components update
gcloud config set run/region us-central1
gcloud config set project clima-316917
gcloud run deploy clima-test --source .

The app is up and running at this URL https://clima-test-oiook3g2yq-uc.a.run.app/ however it is not loading up despite the app was correctly deployed. See the screenshot below.

image

To be honest I am not sure what I need to fix next. If you want i can add you as a viewer to Google Cloud Run so you can have a look at the logs.

@mccalluc
Copy link
Contributor Author

mccalluc commented Dec 6, 2023

Hmm: I don't know either. For right now, I'd suggest putting this on the back burner.

For debugging this, maybe it would make sense for me to sign up for the free level with google, and try running the workflow from my fork. That would save you from elevating my privs, and give me a sandbox where I wouldn't need to worry about clobbering the real site.

I haven't touched google cloud before, so there's plenty to learn there, and there's also work to be done here, without even thinking about this deployment issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker-less, Python-only deployment to gcloud?
2 participants