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

Remove duplicated local census records on deployment #3829

Conversation

Senen
Copy link
Member

@Senen Senen commented Nov 8, 2019

References

Related Pull Requests: #3646

PR #3646 includes a migration that adds an unique database constraint to LocalCensusRecord model so we need to provide a way to remove duplicated records before to apply the new index to database.

Objectives

Remove duplicated local census records automatically during capistrano deployment and before applying new aforementioned migration.

Also provide a rake task to remove duplicated records manually for those Consul installations that are not using caspistrano.

Release notes ⚠️

With this PR people using capistrano may not worry about deleting local census records dupicates manually from the database, but people using other deployment strategies that capistrano, for example Heroku, should manually run the rake task local_census_records:remove_duplicates before running db:migrate.

Production server test results

Deployment test steps:

  1. Prepare a production server with the Ansible installer and deploy the Consul 1.0.0 version.
  2. Create dups in the LocalCensusRecord database table
  3. Deploy with capistrano the code of this PR
  4. Check during deployment and before the capistrano step deploy:migrate those duplicated records were removed successfully

Below the deployment log that demonstrates this PR is working successfully.

01:02 remove_local_census_records_duplicates
      01 /home/deploy/consul/rvm1scripts/rvm-auto.sh . bundle exec rake local_census_records:remove_duplicates
      01 INFO Removing local census records duplicates
      01
      01 INFO Removed 6 records.
      01
    ✔ 01 [email protected] 4.907s
01:07 deploy:migrate
      [deploy:migrate] Run `rake db:migrate`
01:07 deploy:migrating

@javierm javierm added this to Reviewing in Roadmap via automation Nov 8, 2019
Copy link
Member

@javierm javierm left a comment

Choose a reason for hiding this comment

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

@Senen Thanks a lot! I've left a couple of comments about minor things.

lib/tasks/local_census_records.rake Outdated Show resolved Hide resolved
spec/lib/tasks/local_census_records_spec.rb Outdated Show resolved Hide resolved
spec/lib/tasks/local_census_records_spec.rb Show resolved Hide resolved
spec/lib/tasks/local_census_records_spec.rb Show resolved Hide resolved
spec/lib/tasks/local_census_records_spec.rb Outdated Show resolved Hide resolved
@javierm javierm moved this from Reviewing to Doing in Roadmap Nov 8, 2019
Also supress migration messages during spec execution to keep test log as clean
as possible.
@Senen Senen force-pushed the remove-duplicated-local-census-records-on-deployment branch from 98d87f8 to 15b4ff6 Compare November 8, 2019 13:47
@Senen Senen changed the title [WIP] Remove duplicated local census records on deployment Remove duplicated local census records on deployment Nov 8, 2019
Roadmap automation moved this from Doing to Testing Nov 8, 2019
Copy link
Member

@javierm javierm left a comment

Choose a reason for hiding this comment

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

@Senen Thanks a lot!

@javierm javierm merged commit 487008d into consuldemocracy:master Nov 8, 2019
Roadmap automation moved this from Testing to Release 1.1.0 Nov 8, 2019
smarques pushed a commit to venetochevogliamo/consul that referenced this pull request Apr 29, 2020
…ated-local-census-records-on-deployment

Remove duplicated local census records on deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Roadmap
  
Release 1.1.0
Development

Successfully merging this pull request may close these issues.

None yet

2 participants