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

File Transfer methods put() and upload() don't honor dry-run flag #152

Closed
benissimo opened this issue Feb 8, 2012 · 5 comments
Closed
Labels

Comments

@benissimo
Copy link

https://github.com/capistrano/capistrano/blob/master/lib/capistrano/configuration/actions/file_transfer.rb

transfer() honors the dry-run flag.
upload() (and put() which is a wrapper around upload()) doesn't.

This is why, for example, deploy:web:disable doesn't honor the dry-run flag:
#151

But more generally, any task built on put() or upload() will fail to honor the dry-run flag.

@shtirlic
Copy link
Contributor

All methods like put get upload download are using the transfer method which honors dry-run flag as you could see in this line of code https://github.com/capistrano/capistrano/blob/master/lib/capistrano/configuration/actions/file_transfer.rb#L40
But the execute_on_servers actually connecting to the all servers and will not harm to servers.

@benissimo
Copy link
Author

My mistake, thanks for correcting me. I guess it's the fact that it does actually try to connect to the remote servers which threw me. As you noted, transfer does still invoke "execute_on_servers", even in dry-run. Whereas instead "run()" bails out before calling "execute_on_servers" in dry-run mode. The discrepancy between the two is most noticeable when alternating between deploy:web:enable and deploy:web:disable, both with dry-run. One simply prints out the command it would have tried, the other also attempts to connect to the remote servers. In my opinion it would be a good practice for dry-run to always imply skipping execute_on_servers. I see from the line you cited above that it would not actually try to do anything once connected to the remote servers. Still, I feel a dry run should not even connect to the servers. Alternatively, if connecting to the servers is deemed as something to be included in a dry run, it would be clearer if that occurred in all cases (run() as well as transfer()).

@shtirlic
Copy link
Contributor

Ok, I got your point, I also think that result should be the same in both cases, I''ll try to come up with a patch for the transfer method.

@benissimo
Copy link
Author

Thanks!

@shtirlic
Copy link
Contributor

I submitted my pull request as #167

mattbrictson added a commit to mattbrictson/capistrano that referenced this issue Aug 24, 2016
Add navigation and hopefully fix code block.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants