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

Fix host with multiple role capistrano deployment #281

Closed
wants to merge 2 commits into from

Conversation

ajwalters
Copy link

Bug described here: #266

When a host belongs to multiple roles which have separate cron tasks
associated, the last role specified in whenever_roles overwrites the
previous crontab.

Fix is to write crontab one host at a time, generating all roles for
that host

Bug described here: javan#266

When a host belongs to multiple roles which have separate cron tasks
associated, the last role specified in whenever_roles overwrites the
previous crontab.

Fix is to write crontab one host at a time, generating all roles for
that host
role_arg = " --roles #{role}"
end
server_roles_map.each do |server, roles|
roles_arg = "" # empty for begin
Copy link
Owner

Choose a reason for hiding this comment

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

Is setting this empty string necessary now that you have the line to follow?

@javan
Copy link
Owner

javan commented Dec 14, 2012

Thanks for working on this! I would LOVE it if you could add a comment or two. Future developers will thank you.

cc @cap10morgan

@ajwalters
Copy link
Author

Comment to the README?

@javan
Copy link
Owner

javan commented Dec 14, 2012

In recipes.rb

@cap10morgan
Copy link
Contributor

Thanks for working on this, @ajwalters. We need some tests around this functionality. I'm working on that locally and will issue a pull request once it's ready.

There are a few tricky backwards compatibility issues here.

@ajwalters
Copy link
Author

Yeah, I tried to look into testing this but it seemed as if the capistrano integration lacked any tests. Not saying that's a good thing, but seemed like a larger refactoring to make the Capistrano integration more testable. Is this the approach you're taking?

@cap10morgan
Copy link
Contributor

@ajwalters Similar to that, yes. Since we're using Test::Unit here instead of RSpec, the testing library mentioned in that post isn't applicable. But I am pulling the code out of recipe.rb and defining a separate, testable module that it will include instead.

@javan
Copy link
Owner

javan commented Dec 14, 2012

❤️

@javan javan closed this Dec 14, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants