capistrano-pending functionality doesn't work unless db role is defined

[erikhansen]

When a db role is not defined, the capistrano-pending functionality doesn't work. For example, if you're using a role configuration like this:

role :app, %w{
  www-data@example.com
}

You'll get output like this:

However if you setup both the app and db roles like this:

server 'example.com', user: 'www-data', roles: %w{app db}

You'll get output like expected:

Forcing users to define a db role doesn't seem like the right approach, so I'll let you take it from here to come up with the proper approach to this bug as I don't have any more time to look into this.

This seems to be a bug caused by the Capistrano :revision variable (see here) not being populated unless a db role is defined. If you agree that this seems to be a core Capistrano bug, not anything related to this Gem, then you're free to close this issue. We may just want to update the relevant section of this Gem's readme to alert users of this bug and open an issue on the Capistrano issues list.

[davidalger]

Very good findings, thanks for sharing! Your notes about it being related to the :db role not being defined definitely got me on the right track.

The capistrano-pending gem looks to :capistrano_pending_role for the role to lookup the current revision on. If that var is unset, it defaults to :db. So technically not a bug in this gem, certainly not playing nice in all cases, as it would require configuring that gem as well.

I've made changes in the above commit to remedy the issue. In short, I'm setting that role to match the :magento_deploy_setup_role by default, but still allowing it to be overridden.

Now all of this said… I've been refactoring this code some more, as I was unraveling more issues with how the underlying functionality works (this is why back when you initially PR'd it I didn't want it enabled by default…I feared it would work inconsistently). For example:

• If REVISION on the server is ahead of the commit being deployed, nothing is reported as being deployed, and the current implementation actually tells the user that the from and to are the same when they are in-fact, definitively NOT the same.

• Oddities in how the revisions are reported, as the current revision is using the lengthy hash, but the deployed ref is using the symbolic name

• Potential to incorrectly report pending changes on a multiple node deployment

In my refactoring, I'm discovering there really is very little in the 3rd-party gem which is not being done natively, so very well may remove it as a dependency by the time I'm finished ;)

[erikhansen]

Thanks for spending the time to make fix those issues.

[davidalger]

Deploying to multiple hosts which no current deployment:

Deploying even changes to multiple hosts:

Deploying a revision behind what's currently deployed:

Deploying a revision behind what's currently deployed (on single host to create disparity for future test run):

Deploying to multiple hosts which with divergent revisions:

No change deployment with the warning; stopped via 'n' input:

Test with one host having nothing to deploy (warning should not show, with deployment continuing automatically):

No change deployment with warning; proceeding via 'y' input:

Just a simple deploy:pending to check on system works (with no warning since it's not a deploy):

Adding the following to project configuration will disable warning that will show on deployments with no pending changes:

set :magento_deploy_pending_warn, false

There are a couple other default values such as the log format and default role to use set in the defaults.rb as well.

One fringe case which I have not accounted for (and I'm not sure this is a way to account for it very readily): If one is deploying to a different branch than currently deployed, the change log won't show what changes on the current branch are being removed from the server, which could affect deployments where users dynamically change the feature branch being deployed to the same target. An example is the following where the latest commit on the 2.1 branch is currently deployed and cap is asked to deploy from a tag on the 2.0 branch. In that case, the commits from the 2.0 branch will be printed, but the fact that whats currently on the server from the 2.1 branch is ignored:

This should all be rolled out in the next release.

[erikhansen]

Wow, this looks fantastic! Good work on accounting for all of those scenarios.

In regards to the fringe case you mentioned: if the 2nd point in this issue were to be implemented, it seems like it would solve for that use case, as it would warn the user that the deployed branch was being changed and they could do their due diligence to determine the impact of switching branches. I agree with your latest comment on that issue that creating the functionality as a separate Gem might make more sense. My team is managing an M1 project right now and they're frequently deploying different branches and it would be great to be able to pull that functionality into that project.