forumone / web-starter

Starting place for developing Drupal, Wordpress and other web applications
http://forumone.github.io/web-starter/
22 stars 11 forks source link

"Database: Connected" removed in drush 7.x+ and causes database backups to fail #250

Closed johnbburg closed 8 years ago

johnbburg commented 8 years ago

The script in drush.rake performs a check to see if it has a connection to the database before it attempts to generate a backup dump. This check is dependent on the following line being output in the $ drush status command:

 Database                        :  Connected 

This works fine with drush 6.x, However, in drush 7.x, this line seems to have been removed. Which means that for any site using continuous integration, which also uses drush 7 or greater, the test in drush.rake fails, and database backups are not generated prior to deployment.

johnbburg commented 8 years ago

Here is the commit where that 'Connected' status line was removed: https://github.com/drush-ops/drush/commit/0f4e59a9ceb1c95fdd4dfff925cc131fec9d83c1#diff-9e2b1896af01f89d4d97823474f12ae2

azinck commented 8 years ago

Shouldn't the build also then fail so as to avoid silently not taking backups?

wwhurley commented 8 years ago

The first deployment will not have a valid settings file so won't be able to connect to the database. So there is a case where we would want the deployment to proceed without a valid DB connection.

azinck commented 8 years ago

Ah, a tricky one.

azinck commented 8 years ago

Perhaps initial deployment should be kicked off in a more manual fashion and use a different script entirely. Subsequent CI deployments should use a script that fails on critical things like this.

wwhurley commented 8 years ago

Right now the backup happens during the deploy:publishing stage which is before some of the other stages we need to actually deploy the files. We'd have to do some extensive restructuring of our Rake tasks to separate it out. It's probably the right solution, but that will likely have to wait until we change things to work with Capistrano 3.5+.

johnbburg commented 8 years ago

Just to note for anyone looking for a quick fix to this issue. Just remove

'Connected' == status['db-status'] &&

from this line in drush.rake: https://github.com/forumone/web-starter/blob/1.1.x/lib/capistrano/tasks/drush.rake#L111.

johnbburg commented 8 years ago

I wonder if this was removed from drush in error. The linked commit's comments seem irrelevant to the removal of that line. Maybe we should handle this by filing an issue over at https://github.com/drush-ops/drush/issues.

johnbburg commented 8 years ago

The folks over at the drush project responded quickly to re-add the connection status. https://github.com/drush-ops/drush/issues/2372. So this should be fixed in drush versions 8.1.6 or greater. @mshade @jryden @wilsontech Just wanted to make sure you see this.