[ARVADOS] updated: 8072742e6557966b3e3d699e18fd3ee8656ed5b7

git at public.curoverse.com git at public.curoverse.com
Tue Apr 22 14:27:27 EDT 2014


Summary of changes:
 services/api/app/models/pipeline_instance.rb       |   22 +++++----
 services/api/app/models/user.rb                    |    2 +-
 .../20140422011506_pipeline_instance_state.rb      |    4 +-
 services/api/test/unit/pipeline_instance_test.rb   |   49 ++++++++++++++++++--
 services/api/test/unit/user_test.rb                |   44 ++++++++++++++++++
 5 files changed, 104 insertions(+), 17 deletions(-)

       via  8072742e6557966b3e3d699e18fd3ee8656ed5b7 (commit)
       via  007a1e6edf9125990e286c71b00f51405470a4a6 (commit)
       via  c92d3fcdc2ab6bc3ec9bb03567de9203ff9b1f79 (commit)
       via  3aa0915d0b9b0affd9a784df3d138f0d13df303b (commit)
      from  5cb205c567c312345376bcd2b7104075b5710d7f (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 8072742e6557966b3e3d699e18fd3ee8656ed5b7
Merge: 007a1e6 3aa0915
Author: radhika chippada <radhika at curoverse.com>
Date:   Tue Apr 22 14:24:55 2014 -0400

    Merge branch 'master' into 2352-phased-pipeline-instance-state


commit 007a1e6edf9125990e286c71b00f51405470a4a6
Author: radhika chippada <radhika at curoverse.com>
Date:   Tue Apr 22 14:00:20 2014 -0400

    Use the renamed components_look_ready? method during migration.

diff --git a/services/api/db/migrate/20140422011506_pipeline_instance_state.rb b/services/api/db/migrate/20140422011506_pipeline_instance_state.rb
index a3bc7fe..cc153b9 100644
--- a/services/api/db/migrate/20140422011506_pipeline_instance_state.rb
+++ b/services/api/db/migrate/20140422011506_pipeline_instance_state.rb
@@ -18,7 +18,7 @@ class PipelineInstanceState < ActiveRecord::Migration
           if pi[:active] == true
             pi.state = PipelineInstance::RunningOnServer
           else
-            if PipelineInstance.is_ready pi.components
+            if pi.components_look_ready?
               pi.state = PipelineInstance::Ready
             else
               pi.state = PipelineInstance::New
@@ -34,6 +34,7 @@ class PipelineInstanceState < ActiveRecord::Migration
       end
     end
 
+# We want to perform addition of state, and removal of active and success in two phases. Hence comment these statements out.
 =begin
     if column_exists?(:pipeline_instances, :active)
       remove_column :pipeline_instances, :active
@@ -46,6 +47,7 @@ class PipelineInstanceState < ActiveRecord::Migration
   end
 
   def down
+# We want to perform addition of state, and removal of active and success in two phases. Hence comment these statements out.
 =begin
     add_column :pipeline_instances, :success, :boolean, :null => true
     add_column :pipeline_instances, :active, :boolean, :default => false

commit c92d3fcdc2ab6bc3ec9bb03567de9203ff9b1f79
Author: radhika chippada <radhika at curoverse.com>
Date:   Tue Apr 22 13:48:36 2014 -0400

    Rename is_ready method to components_look_ready? and add additional tests for state change.

diff --git a/services/api/app/models/pipeline_instance.rb b/services/api/app/models/pipeline_instance.rb
index 211b91a..fbe3690 100644
--- a/services/api/app/models/pipeline_instance.rb
+++ b/services/api/app/models/pipeline_instance.rb
@@ -40,17 +40,16 @@ class PipelineInstance < ArvadosModel
   end
 
   # if all components have input, the pipeline is Ready
-  def self.is_ready components
-    if !components || components.empty?  # is this correct?
-      return true
+  def components_look_ready?
+    if !self.components || self.components.empty?
+      return false
     end
 
     all_components_have_input = true
-    components.each do |name, component|
+    self.components.each do |name, component|
       component['script_parameters'].each do |parametername, parameter|
         parameter = { 'value' => parameter } unless parameter.is_a? Hash
-        if parameter['value'].nil? and
-            ![false,'false',0,'0'].index parameter['required']
+        if parameter['value'].nil? and parameter['required']
           if parameter['output_of']
             next
           end
@@ -144,7 +143,7 @@ class PipelineInstance < ArvadosModel
       if self.active
         self.state = RunningOnServer
       else
-        if PipelineInstance.is_ready self.components
+        if self.components_look_ready?
           self.state = Ready
         else
           self.state = New
@@ -160,7 +159,7 @@ class PipelineInstance < ArvadosModel
       end
     elsif state_changed?
       case self.state
-      when New, Ready
+      when New, Ready, Paused
         self.active = false
         self.success = nil
       when RunningOnServer
@@ -172,13 +171,16 @@ class PipelineInstance < ArvadosModel
       when Failed
         self.active = false
         self.success = false
+        self.state = Failed   # before_validation will fail if false is returned in the previous line
       when Complete
         self.active = false
         self.success = true
+      else
+        return false
       end
     elsif components_changed? 
       if !self.state || self.state == New || !self.active
-        if PipelineInstance.is_ready self.components
+        if self.components_look_ready?
           self.state = Ready
         else
           self.state = New
@@ -191,7 +193,7 @@ class PipelineInstance < ArvadosModel
     if !self.state || self.state == New
       if self.active
         self.state = RunningOnServer
-      elsif PipelineInstance.is_ready self.components
+      elsif self.components_look_ready?
         self.state = Ready
       else
         self.state = New
diff --git a/services/api/test/unit/pipeline_instance_test.rb b/services/api/test/unit/pipeline_instance_test.rb
index 2192001..20c833f 100644
--- a/services/api/test/unit/pipeline_instance_test.rb
+++ b/services/api/test/unit/pipeline_instance_test.rb
@@ -12,7 +12,7 @@ class PipelineInstanceTest < ActiveSupport::TestCase
     # save the pipeline and expect state to be New
     pi.save
     pi = PipelineInstance.find_by_uuid 'zzzzz-xxxxx-f4gneyn6br1xize'
-    assert_equal PipelineInstance::Ready, pi.state, 'expected state to be New for new pipeline'
+    assert_equal PipelineInstance::New, pi.state, 'expected state to be New for new pipeline'
     assert !pi.active, 'expected active to be false for a new pipeline'
     assert !pi.success, 'expected success to be false for a new pipeline'
   end
@@ -21,7 +21,7 @@ class PipelineInstanceTest < ActiveSupport::TestCase
     pi = pipeline_instances :new_pipeline
 
     # add a component with no input and expect state to be New
-    component = {'script_parameters' => {"input_not_provided" => {"required" => "true"}}}
+    component = {'script_parameters' => {"input_not_provided" => {"required" => true}}}
     pi.components['first'] = component
     components = pi.components
     pi.update_attribute 'components', pi.components
@@ -31,6 +31,17 @@ class PipelineInstanceTest < ActiveSupport::TestCase
     assert !pi.active, 'expected active to be false after update'
     assert !pi.success, 'expected success to be false for a new pipeline'
 
+    # add a component with no input not required
+    component = {'script_parameters' => {"input_not_provided" => {"required" => false}}}
+    pi.components['first'] = component
+    components = pi.components
+    pi.update_attribute 'components', pi.components
+    pi = PipelineInstance.find_by_uuid 'zzzzz-xxxxx-f4gneyn6br1xize'
+    assert_equal PipelineInstance::Ready, pi.state, 'expected state to be Ready after adding component with input'
+    assert_equal pi.components.size, 1, 'expected one component'
+    assert !pi.active, 'expected active to be false after update'
+    assert !pi.success, 'expected success to be false for a new pipeline'
+
     # add a component with input and expect state to become Ready
     component = {'script_parameters' => {"input" => "yyyad4b39ca5a924e481008009d94e32+210"}}
     pi.components['first'] = component
@@ -56,12 +67,40 @@ class PipelineInstanceTest < ActiveSupport::TestCase
     assert !pi.active, 'expected active to be false after update'
     assert !pi.success, 'expected success to be false for a new pipeline'
 
-    pi.success = true
+    pi.state = PipelineInstance::RunningOnServer
+    pi.save
+    pi = PipelineInstance.find_by_uuid 'zzzzz-xxxxx-f4gneyn6br1xize'
+    assert_equal PipelineInstance::RunningOnServer, pi.state, 'expected state to be RunningOnServer after updating state to RunningOnServer'
+    assert pi.active, 'expected active to be true after update'
+    assert !pi.success, 'expected success to be alse after update'
+
+    pi.state = PipelineInstance::Paused
     pi.save
     pi = PipelineInstance.find_by_uuid 'zzzzz-xxxxx-f4gneyn6br1xize'
-    assert_equal PipelineInstance::Complete, pi.state, 'expected state to be Complete after updating success to true'
+    assert_equal PipelineInstance::Paused, pi.state, 'expected state to be Paused after updating state to Paused'
+    assert !pi.active, 'expected active to be false after update'
+    assert !pi.success, 'expected success to be false after update'
+
+    pi.state = PipelineInstance::Complete
+    pi.save
+    pi = PipelineInstance.find_by_uuid 'zzzzz-xxxxx-f4gneyn6br1xize'
+    assert_equal PipelineInstance::Complete, pi.state, 'expected state to be Complete after updating state to Complete'
     assert !pi.active, 'expected active to be false after update'
     assert pi.success, 'expected success to be true after update'
+
+    pi.state = 'bogus'
+    pi.save
+    pi = PipelineInstance.find_by_uuid 'zzzzz-xxxxx-f4gneyn6br1xize'
+    assert_equal PipelineInstance::Complete, pi.state, 'expected state to be unchanged with set to a bogus value'
+    assert !pi.active, 'expected active to be false after update'
+    assert pi.success, 'expected success to be true after update'
+
+    pi.state = PipelineInstance::Failed
+    pi.save
+    pi = PipelineInstance.find_by_uuid 'zzzzz-xxxxx-f4gneyn6br1xize'
+    assert_equal PipelineInstance::Failed, pi.state, 'expected state to be Failed after updating state to Failed'
+    assert !pi.active, 'expected active to be false after update'
+    assert !pi.success, 'expected success to be false after update'
   end
 
   test "update attributes for pipeline with two components" do
@@ -69,7 +108,7 @@ class PipelineInstanceTest < ActiveSupport::TestCase
 
     # add two components, one with input and one with no input and expect state to be New
     component1 = {'script_parameters' => {"something" => "xxxad4b39ca5a924e481008009d94e32+210", "input" => "c1bad4b39ca5a924e481008009d94e32+210"}}
-    component2 = {'script_parameters' => {"something_else" => "xxxad4b39ca5a924e481008009d94e32+210", "input_missing" => {"required" => "true"}}}
+    component2 = {'script_parameters' => {"something_else" => "xxxad4b39ca5a924e481008009d94e32+210", "input_missing" => {"required" => true}}}
     pi.components['first'] = component1
     pi.components['second'] = component2
     components = pi.components

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list