Skip to content
This repository

It should be possible to continue executing task chain on NoMatchingServersError #51

Merged
merged 1 commit into from over 2 years ago

4 participants

Levent Ali @lebreeze Chris Griego Rob Slifka Lee Hambley
Levent Ali @lebreeze
levent commented June 08, 2011

We find it somewhat illogical to halt executing the task chain when this exception is raised.

There are two ways I would assume this would work.
1) Treat it the same as a task failing and rollback
2) Assume that the reason no servers match is intentional and continue executing the rest of the tasks (which seemed the most logical assumption to us)

Is this a valid issue or am I missing something?

Maybe an option similar to :on_error => :continue should be available to override the default behaviour?

Cheers in advance.

Levent

Levent Ali @lebreeze
levent commented June 08, 2011

After looking at this a bit more in depth, it seems the rollback doesn't occur because our case was after deploy so outside the transaction.

I understand the default behaviour makes sense if say rake db:migrate didn't run because no servers were matched. In that case we would definitely want a rollback.

However, if it's not currently possible, I see great value in being able to explicitly choose to continue if no servers were matched.

Chris Griego

+1 I like the approach of using a new option. That way it can be set at a command or task level.

Rob Slifka

+1 I also like a new option. Bonus would be a new global setting rather than a per-task option. That way I can tilt the default in the direction that makes the most sense and override when necessary (i.e. it may be that I want all of them to continue on error save for one).

Lee Hambley
Owner

I've personally never had a use for this, but I'll gladly take a patch, if it's really a bad itch for some of you guys, as long as there are tests, I will absolutely merge it.

Rob Slifka

@leehambley - We have a dozen roles in our deploy environment. When testing smaller parts of that environment in a different cap stage, we often don't setup servers for all 12 roles; instead setting up just 1 server for focused testing. This means we have to comment out major ports of the script to be able to do focused development. There may be another way to work here but for now the idea behind this patch seems to be the most obvious to us :) Thanks!

Chris Griego

I added a pull request.

Lee Hambley
Owner
Lee Hambley leehambley merged commit edf7092 into from August 02, 2011
Lee Hambley leehambley closed this August 02, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
3  lib/capistrano/configuration/actions/invocation.rb
@@ -111,6 +111,9 @@ def invoke_command(cmd, options={}, &block)
111 111
         # * :except - specifies a condition limiting which hosts will be selected to
112 112
         #   run the command. This is the inverse of :only (hosts that do _not_ match
113 113
         #   the condition will be selected).
  114
+        # * :on_no_matching_servers - if :continue, will continue to execute tasks if
  115
+        #   no matching servers are found for the host criteria. The default is to raise
  116
+        #   a NoMatchingServersError exception.
114 117
         # * :once - if true, only the first matching server will be selected. The default
115 118
         #   is false (all matching servers will be selected).
116 119
         # * :max_hosts - specifies the maximum number of hosts that should be selected
7  lib/capistrano/configuration/connections.rb
@@ -153,7 +153,7 @@ def execute_on_servers(options={})
153 153
           servers = find_servers_for_task(task, options)
154 154
 
155 155
           if servers.empty?
156  
-            if ENV['HOSTFILTER']
  156
+            if ENV['HOSTFILTER'] || task.options.merge(options)[:on_no_matching_servers] == :continue
157 157
               logger.info "skipping `#{task.fully_qualified_name}' because no servers matched"
158 158
               return
159 159
             else
@@ -167,7 +167,10 @@ def execute_on_servers(options={})
167 167
           end
168 168
         else
169 169
           servers = find_servers(options)
170  
-          raise Capistrano::NoMatchingServersError, "no servers found to match #{options.inspect}" if servers.empty?
  170
+          if servers.empty?
  171
+            raise Capistrano::NoMatchingServersError, "no servers found to match #{options.inspect}" if options[:on_no_matching_servers] != :continue
  172
+            return
  173
+          end
171 174
         end
172 175
 
173 176
         servers = [servers.first] if options[:once]
27  test/configuration/connections_test.rb
@@ -189,6 +189,11 @@ def test_execute_on_servers_without_current_task_should_raise_error_if_no_matchi
189 189
     assert_raises(Capistrano::NoMatchingServersError) { @config.execute_on_servers(:a => :b, :c => :d) { |list| } }
190 190
   end
191 191
 
  192
+  def test_execute_on_servers_without_current_task_should_not_raise_error_if_no_matching_servers_and_continue_on_no_matching_servers
  193
+    @config.expects(:find_servers).with(:a => :b, :c => :d, :on_no_matching_servers => :continue).returns([])
  194
+    assert_nothing_raised { @config.execute_on_servers(:a => :b, :c => :d, :on_no_matching_servers => :continue) { |list| } }
  195
+  end
  196
+
192 197
   def test_execute_on_servers_should_raise_an_error_if_the_current_task_has_no_matching_servers_by_default
193 198
     @config.current_task = mock_task
194 199
     @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns([])
@@ -198,7 +203,27 @@ def test_execute_on_servers_should_raise_an_error_if_the_current_task_has_no_mat
198 203
       end
199 204
     end
200 205
   end
201  
-  
  206
+
  207
+  def test_execute_on_servers_should_not_raise_an_error_if_the_current_task_has_no_matching_servers_by_default_and_continue_on_no_matching_servers
  208
+    @config.current_task = mock_task(:on_no_matching_servers => :continue)
  209
+    @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns([])
  210
+    assert_nothing_raised do
  211
+      @config.execute_on_servers do
  212
+        flunk "should not get here"
  213
+      end
  214
+    end
  215
+  end
  216
+
  217
+  def test_execute_on_servers_should_not_raise_an_error_if_the_current_task_has_no_matching_servers_by_default_and_command_continues_on_no_matching_servers
  218
+    @config.current_task = mock_task
  219
+    @config.expects(:find_servers_for_task).with(@config.current_task, :on_no_matching_servers => :continue).returns([])
  220
+    assert_nothing_raised do
  221
+      @config.execute_on_servers(:on_no_matching_servers => :continue) do
  222
+        flunk "should not get here"
  223
+      end
  224
+    end
  225
+  end
  226
+
202 227
   def test_execute_on_servers_should_determine_server_list_from_active_task
203 228
     assert @config.sessions.empty?
204 229
     @config.current_task = mock_task
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.