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

Task Continuations: A New Feature for Parameterizing Task Execution #243

Closed
wants to merge 9 commits into from
Closed

Conversation

carsomyr
Copy link
Contributor

Firstly, apologies for the huge pull request. I am proposing a fairly extensive feature, so I needed to make sure that it was fully tested and documented, hence the numerous commits, files modified, and documentation.

Introduction

The motivation behind task continuations is to provide a way to alter the behavior of tasks without rewriting them. A few projects (the multistage extension, capistrano-multiconfig, capistrano-multiproject, CapHub) already do various aspects of this, albeit without modification of Capistrano itself. The implementation strategy in each case involves executing multiple tasks and having later tasks reflect on the results of earlier tasks. It invariably presumes some sort of implicit dependency among tasks.

After thinking about how the current plugins for multi{config, stage, project} worked, I decided to take a different tack: Tasks would remain independent, but they could be chained in a recursive manner. That's the essence of a task continuation (taking after the functional programming notion of continuation): It's like a normal task in all respects save for the block parameter in the task body, which represents a deferred computation.

# This is a normal task.
task :t1 do
  puts "Executing the main task."
end

# This is a task continuation.
task :tc1 do |&block|
  puts "Executing pre-task code."
  block.call
  puts "Executing post-task code."
end

By applying potentially multiple, nested task continuations to the main task (achieved with a natural extension of the Capistrano command line and DSL syntax), it's not hard to imagine how the main task's environment could be modified to one's liking. In the example above, executing any of the snippets below would result in the desired output.

$ cap tc1:t1

or

$ cap tc1::t1

or

task :t2 do
  tc1.t1
end

$ cap t2

outputs

  * executing the task continuation `tc1'
Executing pre-task code.
  * executing the task `t1'
Executing the main task.
Executing post-task code.

The above example is just a taster. The next section details the documentation and examples dedicated to helping you better understand the proposed feature.

Documentation and Examples

I've added a wiki page at my fork containing the full task continuations specification. It contains more in-depth discussion with regards to usage. Perhaps more importantly, the Capistrano-MultiEnv project serves as a demonstration of the expressive power of task continuations and helps me with my own deployment needs. By following the instructions in the README, you'll be able to see how task continuations can be used to parameterize multi{cloud, stage, project} deployments.

Logistical Details

  1. I am looking for the broader community's feedback on what they like and what can be improved.
  2. I believe that the feature is nearly backwards compatible. An area of trouble could be with DSL usage, since DSL methods now return NamespaceContext objects rather than Namespace objects. Practically speaking, few users should be affected, as most invoke the DSL to execute tasks within tasks.
  3. @leehambley, what additional steps should I take to guide a pull request of this size smoothly through the review process?

@leehambley
Copy link
Member

Hi @carsomyr I think the commits should be squashed when they are merged, that's point #1, otherwise - there's nothing that scares me too much about this.

I guess you missed a mailing list post about how I am resigning from Capistrano for the next few months, based on your contribution, I think you would be an ideal person to take over maintainership in the short term, shepherd your patch to production, and kill a few of the other PRs if you are interested.

If you enjoy the work, I could imagine resigning full-time, and allowing you to take the project in the direction you desire.

If you prefer to take the discussion off-line, please contact me (http://lee.hambley.name has my details) otherwise I'd be happy to continue the discussion here.

My first reaction is "ohh god, don't touch the DSL" - I think that Capistrano's DSL is one of our worst features, it's rake-ish, but not powerful enough to do what people need, with Thor and Rake being such pervasive, well written (ok, maybe not Thor, it's been a long time since I used it) pieces of software, I really believe we ought to be making the most of these tools, rather than extending our own (broken by design, in my opinion) DSL.

Capistrano is an older tool now, and Thor and Rake weren't as pervasive as they are now when it was written, but as the ecosystem around Ruby has evolved, I think it's time for a significant overhaul of Capistrano's structure, and to the same end, feature set.

That all being said, I admit the need for this kind of behaviour, even if i'm not convinced by the implementation (having grown quite used to solving these problems myself in other ways) - and this patch is certainly less extreme than expecting those suffering pain points to upgrade to a vaporware new-version.

@carsomyr
Copy link
Contributor Author

@leehambley, thanks for the vote of confidence. As I see it, I am not quite ready to take over, given that I've only explored half of Capistrano. I am, however, looking forward to closing some core functionality, non-deploy recipe related PRs. Just give me commit access, and I'll start working upwards from the least controversial, smallest PRs to the largest ones.

If Capistrano's DSL is indeed one of the worst features, we should make a list of things "wrong" about it. I'll kick off by saying that the method_missing strategy may not be the best way to go, and some object reorientation may be required (proper division of functionality betweeen Configuration and Namespace objects). Namespaces should also be able to "remember" where they are. Otherwise, you'd get an absurd situation like the one below.

namespace :ns1 do
  namespace :ns2 do
  end
end

task :t1 do
  ns1.ns2.ns1.ns2.ns1
end

Are there any major contributors I should be aware of? Perhaps they could be brought in on this conversation?

@leehambley
Copy link
Member

@carsomyr added you with commit access a few minutes ago, would be happy to engage with you on any of the pull requests, and naturally enough grant rubygems access when you feel confident enough.

Are there any major contributors I should be aware of? Perhaps they could be brought in on this conversation?

@cgriego is the contributor of the asset pipeline tasks, there's some outstanding problems with them. Also @rgo is interested in contributing, but lacks the time to commit (as I do).

@carsomyr
Copy link
Contributor Author

@leehambley Thanks. I'll start reviewing PRs and bouncing each one off of you to see if I'm vetting them correctly.

@leehambley
Copy link
Member

Thanks @carsomyr - I really appreciate it, I'll do my absolute best to support you with my knowledge so far - please keep in mind that I like to make features work hard to be implemented (as a self-defense mechanism against how much free time I can offer) (http://gettingreal.37signals.com/ch05_Start_With_No.php)

@carsomyr
Copy link
Contributor Author

@leehambley A procedural question for you: For PR merging, I see that many if them were done using the GitHub GUI button. For manual merges, do you prefer that the "Merge pull request ..." message be preserved? Also, do you prefer merge history to be preserved in the case of fast forwards?

@leehambley
Copy link
Member

Roy, I'm actually just doing whatever is faster - I would like to find the git incantation to co-author commits, so that whoever merged is marked as the 2nd author of the commit - Rails seems to do this quite nice.

Lee Hambley
Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

On Wednesday, July 25, 2012 at 8:13 AM, Roy Liu wrote:

@leehambley A procedural question for you: For PR merging, I see that many if them were done using the GitHub GUI button. For manual merges, do you prefer that the "Merge pull request ..." message be preserved? Also, do you prefer merge history to be preserved in the case of fast forwards?


Reply to this email directly or view it on GitHub:
#243 (comment)

@carsomyr
Copy link
Contributor Author

carsomyr commented Aug 7, 2012

@leehambley Who owns http://capify.org?

@leehambley
Copy link
Member

I do, needs a change?

Lee Hambley
Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

On Tuesday, August 7, 2012 at 3:37 AM, Roy Liu wrote:

@leehambley (https://github.com/leehambley) Who owns http://capify.org?


Reply to this email directly or view it on GitHub (#243 (comment)).

@carsomyr
Copy link
Contributor Author

carsomyr commented Aug 7, 2012

Yeah. The wiki has a new location. https://github.com/capistrano/capistrano/wiki

@leehambley
Copy link
Member

I'm on vacation this week Roy, I'll be in touch soon - it's capify.rb
forwards to capistranorb.com -> which forwards to the wiki... I believe
it's GoDaddy's forwarding service, but I don't remember off the top of my
head, and will fix it when I'm back near a computer for more than 5 minutes.

  • Lee

@ghost ghost assigned carsomyr Aug 16, 2012
@travisbot
Copy link

This pull request fails (merged be4584cf into bc577df).

@carsomyr
Copy link
Contributor Author

@leehambley Why is the Travis bot monitoring my fork? I pushed to my own repository, not capistrano/capistrano.

@leehambley
Copy link
Member

@carsomyr something to do with you having cloned the .travisci file in the repository? I'm not toally certain how that works, but that'd be my first guess.

@carsomyr
Copy link
Contributor Author

@leehambley See this Travis blog entry. I can see the rationale; it's still annoying because the pull request should be my own experimental domain before it's vetted and merged.

@leehambley
Copy link
Member

I'd tend to agree @carsomyr - helpful for many cases "it's not just passing for me, but also at travis" certainly adds credibility to pull requests.

@travisbot
Copy link

This pull request fails (merged ebf50de6 into 3632188).

@travisbot
Copy link

This pull request fails (merged beda6c62 into 30a942c).

@leehambley
Copy link
Member

Hey @carsomyr what was your thoughts on this in the end, did you want to wait for v3 to solve this for you, or did you want to try and get it into v2? (closing pending your answer, but please feel free to reopen)

@leehambley leehambley closed this Apr 2, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants