thoughtbot / parity

Shell commands for development, staging, and production parity for Heroku apps
https://thoughtbot.com
MIT License
895 stars 57 forks source link

Integrate with Rails 5.x destructive action on production guards #147

Closed seanpdoyle closed 6 years ago

seanpdoyle commented 6 years ago

What command did you execute?

After restoring a local database from production with:

❯ development restore-from production 

I tried to drop the database:

❯ rails db:drop

What did you expect to happen?

After cloning a database from production to development, I expected to be able to interact without any precautionary guards.

What actually happened?

With the advent of some Rails guards against dropping production, rails db:drop failed with some helpful warnings.

❯ development restore-from production 
# ...
❯ rails db:drop
rails aborted!
ActiveRecord::ProtectedEnvironmentError: You are attempting to run a destructive action against your 'production' database.
If you are sure you want to continue, run the same command with the environment variable:
DISABLE_DATABASE_ENVIRONMENT_CHECK=1
# ...
Tasks: TOP => db:drop => db:check_protected_environments

After running the Rails command to set the database environment:

❯ rails db:environment:set RAILS_ENV=development

I was able to drop the database without error or warning:

❯ rails db:drop

Some information about your installation

macOS Sierra 10.12.6

~/src
❯ which development
/usr/local/bin/development

~/src
❯ which staging
/usr/local/bin/staging

~/src
❯ which production
/usr/local/bin/production
❯ brew list parity
/usr/local/Cellar/parity/2.4.0/bin/development
/usr/local/Cellar/parity/2.4.0/bin/production
/usr/local/Cellar/parity/2.4.0/bin/staging
/usr/local/Cellar/parity/2.4.0/lib/parity/ (5 files)
/usr/local/Cellar/parity/2.4.0/lib/parity.rb
geoffharcourt commented 6 years ago

Hi @seanpdoyle! I think this would be a great scenario to handle.

The only reason I've held off so far is that resolving it by setting the DB value that removes the check would make the Parity DB commands Rails-specific.

Maybe we could always run with DISABLE_DATABASE_ENVIRONMENT_CHECK=1 for the drop command? I worry about somehow blowing someone's production DB away if they accidentally had DATABASE_URL set in their .env file.

seanpdoyle commented 6 years ago

@geoffharcourt that makes a lot of sense.

I grepped for Rails in the codebase and found some code related to migrating after a deployment. Other than that, there isn't any local-environment focussed Rails code.

This also might be a non-issue in later versions of Rails. The warnings in the issue itself are on a 5.0.x project. They differ from a 5.2.x project in that the later version of Rails prompts you to run rails db:environment:set RAILS_ENV=development (and even shares a copy-paste ready snippet).

With the older version, seeing You are attempting to run a destructive action against your 'production' database. (which was wrong and misleading) without a path forward or reassurance that I wasn't bring the site down was surprising, alarming, and stressful (!).

It would be nice if parity were smart enough to run that command on your behalf, but at least in the later version's failure scenario, there's a clear path forward.

geoffharcourt commented 6 years ago

My prior attempt to resolve this issue in a way that wouldn't adversely affect non-Rails (or old Rails) apps was to delete the AR metadata table if it was present, but that also causes a warning. Not sure what the best approach is now, maybe we could update the row in the table if it exists?

geoffharcourt commented 6 years ago

Update: we're going to remove migrations from the deploy command in 3.0.0 (allowing users to opt-in to that behavior and get better results via Heroku's release phase feature, so if this is a breaking change, I'd love to do it now to be least disruptive to users' expectations.