-
Notifications
You must be signed in to change notification settings - Fork 2
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
Heroku deployment configuration #105
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #105 +/- ##
=======================================
Coverage 46.01% 46.01%
=======================================
Files 24 24
Lines 3710 3710
=======================================
Hits 1707 1707
Misses 2003 2003 ☔ View full report in Codecov by Sentry. |
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.
I would rather not use auto
at all. Instead, deployment should be triggered by any merge of a PR that does not have the "skip deployment" label (See here for a similar trigger definition).
.github/workflows/deploy.yml
Outdated
run: heroku plugins:install heroku-builds | ||
|
||
- name: Create Heroku Build | ||
run: heroku builds:create --app dandidav |
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.
Is this deploying the entire repository, including the whole of the target/
directory? Can we not just deploy only the target/release/dandidav
asset?
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.
Unfortunately the heroku builds
plugin doesn't allow specifying a single asset. And additionally, you have to include the Procfile
.
I didn't realize how many files got stored in the target/
directory until I just ran the compilation locally, and I agree it's not ideal to push all those artifacts to Heroku. I tried getting around it by storing things in a temporary directory and pushing that - 6b8df2c
Set them to what values? |
@mvandenburgh Also, please enable caching of dependencies etc. like is done in |
Co-authored-by: John T. Wodder II <[email protected]>
Co-authored-by: John T. Wodder II <[email protected]>
Instead, deployment is triggered by any merge of a PR that does not have the "skip deployment" label.
Limits the files pushed to Heroku to the `Procfile` and the `dandidav` binary. Some extra steps are done in the GitHub Action because the `heroku builds:create` command only allows specifying a directory, not an individual file.
I thought we needed someone with admin permissions to set those, but it appears my credentials suffice. So this is all set now |
@jwodder I believe I addressed all your feedback, PTAL and let me know if you have any further questions and/or change requests. |
Also, here is an example of this new workflow running successfully on a different branch - https://github.com/mvandenburgh/dandidav/actions/runs/8273558324/job/22637534296 |
Co-authored-by: John T. Wodder II <[email protected]>
Co-authored-by: John T. Wodder II <[email protected]>
This PR overhauls the current dandidav deployment process. Currently, we have Heroku configured in a "pull" model, i.e. Heroku constantly pulls our repo and reacts to new commits on
main
by doing a new deployment. This PR changes this to a "push" model, where we instead explicitly build and deploy the application on push tomain
via the Heroku CLI in a GitHub Action. Note, a side effect of this is we have complete control over the environment since we're runningcargo build
in the GitHub Action as opposed to on Heroku; thus, the commit hash issue is no longer a problem.It contains two components:
Procfile
for HerokuOther things that need to be done before merging this PR:
Merge the dandi-infrastructure PR removing the Rust buildpack (Disable Rust buildpack and dyno-metadata feature dandi-infrastructure#170)Set theHEROKU_API_KEY
andHEROKU_EMAIL
secrets on thedandi/dandidav
repo