Skip to content
This repository was archived by the owner on Jul 24, 2020. It is now read-only.

[1074] ITS deployment #1270

Merged
merged 1 commit into from
Sep 20, 2015
Merged

[1074] ITS deployment #1270

merged 1 commit into from
Sep 20, 2015

Conversation

orenyk
Copy link
Contributor

@orenyk orenyk commented Jun 2, 2015

Resolves #1074, finishes setting up Capistrano for deployment.

@orenyk orenyk force-pushed the 1074_its_deployment branch from 82646e3 to 2bcddd6 Compare June 2, 2015 18:26
@orenyk
Copy link
Contributor Author

orenyk commented Jun 2, 2015

Ok, squashed and ready for review!

gem 'capistrano-bundler', '~> 1.1.4'
gem 'capistrano-rails', '~> 1.1.2'
gem 'capistrano-rvm', '~> 0.1.2'
gem 'capistrano', '~> 3.3.5', require: false
Copy link
Contributor

Choose a reason for hiding this comment

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

🔥 Extra space here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch.

@orenyk orenyk force-pushed the 1074_its_deployment branch 2 times, most recently from 8958ef8 to 1576ee0 Compare September 13, 2015 11:29
@orenyk
Copy link
Contributor Author

orenyk commented Sep 13, 2015

Just updated dotenv-rails, moved it to the default Bundler group, and removed dotenv-deployment as mentioned in my last comment.

@orenyk orenyk force-pushed the 1074_its_deployment branch from 1576ee0 to 6602cc7 Compare September 16, 2015 06:13
@orenyk
Copy link
Contributor Author

orenyk commented Sep 17, 2015

We've dealt with a bunch of issues on the ITS side but have finally run into an interesting bug. Since we're updating very old databases there are a bunch of migrations to run, including the EquipmentObject --> EquipmentItem migration followed (later) by the Reservation.approval_status removal migration, which does a bunch of processing on existing reservations to convert any information from that column into either status, overdue, or a tag.

Two issues:

  1. Apparently it's really bad practice to use Rails models when touching data in migrations, due to the strong possibility of things getting screwed up later (see here). Apparently good practice is to write SQL statements directly or create local versions of the model, so we should keep that in mind for the future.
  2. More problematically, this migration is failing when it tries to save the reservations since it's running validations, more specifically matched_item_and_model which tries to check and make sure that the reservation has been assigned an equipment item, i.e. has been checked out. However, Rails somehow knows that the original association was for an equipment_object (even though the model file has changed), so it's trying to read the equipment_objects table, which doesn't exist and the migration fails.

Ideally, I wish there were a way to "reload" Rails after running a table rename migration so that all models and associations were up-to-date, but this is also a kludge-y fix for bad code 😞. Take-home number 1 is that we shouldn't rely on Rails models in our migrations anymore. Luckily, this migration calls save! at the end so editing to call save!(validate: false) shouldn't violate the principle of never editing migrations too badly and should resolve the issue in this case. I'll go ahead and do that now.

@orenyk orenyk force-pushed the 1074_its_deployment branch from 6602cc7 to 9f4620a Compare September 17, 2015 06:07
@orenyk
Copy link
Contributor Author

orenyk commented Sep 17, 2015

Also, for posterity, we ran into #1282 and I forgot that I had already fixed it so there was temporarily a duplicate fix in this branch. I rebased onto master and took the fix from there.

@orenyk orenyk force-pushed the 1074_its_deployment branch from 9f4620a to 6649569 Compare September 17, 2015 06:08
@orenyk
Copy link
Contributor Author

orenyk commented Sep 17, 2015

Woo hoo, it is fixed! Man, going back to the v3.4.x codebase makes me really appreciate all the work we've done, particularly with rake db:seed (thanks @squidgetx!). I've now seeded a database from v3.4.10, had to manually make sure that every reservation with an equipment item had been checked out (since this was not guaranteed and would cause one of our migrations to fail), and then checked out this branch and successfully ran rake db:migrate. Hopefully this will be the last bug we have to fix for ITS!

@orenyk
Copy link
Contributor Author

orenyk commented Sep 20, 2015

We've been asked to add an environment variable to disable ALL e-mails for DEV deployments; I'm adding this and using it to optionally set config.action_mailer.perform_deliveries = false in production.rb.

EDIT Done and updated the wiki!

Resolves #1074
- finalize deploy script setup
- finalize custom tasks for deployment
- move custom tasks to lib/capistrano/tasks directory
- update dotenv to 2.0.2; remove dotenv-deployment (deprecated)
- fix broken migration by bypassing Reservation validations
- add environment variable to disable all e-mails (DISABLE_EMAILS)
- other tweaks / fixes
@orenyk orenyk force-pushed the 1074_its_deployment branch from 6649569 to 6998943 Compare September 20, 2015 02:23
@orenyk
Copy link
Contributor Author

orenyk commented Sep 20, 2015

At this point they've got Capistrano deployment working and just have to build the Jenkins wrappers around it, can someone re-review this code so we can merge it into master?

@orenyk
Copy link
Contributor Author

orenyk commented Sep 20, 2015

Ok, @esoterik looked this over and gave it the green light, I'm going to merge it (finally)!

orenyk added a commit that referenced this pull request Sep 20, 2015
@orenyk orenyk merged commit 259d687 into master Sep 20, 2015
@orenyk orenyk deleted the 1074_its_deployment branch January 13, 2016 21:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Finish Capistrano / get Yale ITS deployment set up
2 participants