Move precompile task before migration task #892
Conversation
This change minimises the amount of time between the database migration and switching the old code for the new code, whcih should minimise errors during deployment.
|
Okay, I've done a bit of testing and as predicted; it works fine on regular deployments to existing servers but there are some issues when deploying to a fresh server for the first time, and this kicks in during the precompile step: So we can see from the backtrace that precompiling runs into some legacy assets that use ERB, which call a service, which calls the database and expects there to be some data present (default country) but the data isn't there yet on the first deployment, so it throws a fatal error. Hmmmm okay well we can probably fix this... |
This subset of tasks runs only on the first deployment, and runs before the precompile step (because precompiling will fail unless this has been done at least once). On subsequent deployments, we skip this setup and precompiling is done before the migration step to minimise the time where the database and code are out of sync during the deployment process.
|
Alright, I've added a little fix for first deployments and it seems to be working as intended 🎉 |
| - name: load database schema | ||
| command: bash -lc "bundle exec rake db:schema:load RAILS_ENV={{ rails_env }} I_AM_SURE=1" | ||
| args: | ||
| chdir: "{{ build_path }}" | ||
| tags: | ||
| - rake | ||
| - skip_ansible_lint | ||
|
|
||
| - name: migrate database | ||
| command: bash -lc "bundle exec rake db:migrate RAILS_ENV={{ rails_env }}" | ||
| args: | ||
| chdir: "{{ build_path }}" | ||
| tags: | ||
| - rake | ||
| - skip_ansible_lint | ||
| notify: restart puma | ||
|
|
||
| - name: seed database | ||
| command: bash -lc "bundle exec rake db:seed RAILS_ENV={{ rails_env }}" | ||
| args: | ||
| chdir: "{{ build_path }}" | ||
| tags: | ||
| - seed | ||
| - skip_ansible_lint | ||
| notify: restart puma |
There was a problem hiding this comment.
I think that we can replace these three commands with one:
rails db:setup # Creates all databases, loads all schemas, and initializes with the seed
There shouldn't be any pending migrations but if there were then the migration step is run after the asset compilation anyway.
There was a problem hiding this comment.
This sounds like a good idea, and will be faster with less ansible commands and one less rake task.
But more importantly, it means less code, which will remove the need for a new file :D
There was a problem hiding this comment.
Hmmm that sounds nice and simple, but I'm trying to figure out if we can remove that I_AM_SURE=1 on line 5, or why it's there in the first place. Ironically I am not sure what it does...
There was a problem hiding this comment.
Ah I see it's related to the rails_safe_tasks gem 👍
There was a problem hiding this comment.
It protects from overriding a production db, right? So, we don't actually want that in our deployment. We only want to do safe things during a normal deploy.
There was a problem hiding this comment.
We only want to do safe things during a normal deploy.
I think that's a very good point! We do have a safety check beforehand, but it's checking for a specific table, so not foolproof (eg what if we removed the table one day?).
I agree and think that we should remove I_AM_SURE=1 from deploy.yml, so I think will open a new PR.
There was a problem hiding this comment.
Indeed, I think the db:setup is non-destructive (it only creates if missing). In that case, the I_AM_SURE=1 confirmation should have no effect. I don't know how the old rails_safe_tasks gem handles this, but in any case this isn't dangerous so I won't bother updating it for now.
| - name: load database schema | ||
| command: bash -lc "bundle exec rake db:schema:load RAILS_ENV={{ rails_env }} I_AM_SURE=1" | ||
| args: | ||
| chdir: "{{ build_path }}" | ||
| tags: | ||
| - rake | ||
| - skip_ansible_lint | ||
|
|
||
| - name: migrate database | ||
| command: bash -lc "bundle exec rake db:migrate RAILS_ENV={{ rails_env }}" | ||
| args: | ||
| chdir: "{{ build_path }}" | ||
| tags: | ||
| - rake | ||
| - skip_ansible_lint | ||
| notify: restart puma | ||
|
|
||
| - name: seed database | ||
| command: bash -lc "bundle exec rake db:seed RAILS_ENV={{ rails_env }}" | ||
| args: | ||
| chdir: "{{ build_path }}" | ||
| tags: | ||
| - seed | ||
| - skip_ansible_lint | ||
| notify: restart puma |
There was a problem hiding this comment.
This sounds like a good idea, and will be faster with less ansible commands and one less rake task.
But more importantly, it means less code, which will remove the need for a new file :D
Co-authored-by: David Cook <[email protected]>
Description
Closes #891
This change minimises the amount of time between the database migration and switching the old code for the new code, which should minimise errors during deployment.