[ARVADOS] created: 5233f1d185706095d2f045b8431781c9a421ee16
git at public.curoverse.com
git at public.curoverse.com
Tue Sep 30 18:10:19 EDT 2014
at 5233f1d185706095d2f045b8431781c9a421ee16 (commit)
commit 5233f1d185706095d2f045b8431781c9a421ee16
Author: Tim Pierce <twp at curoverse.com>
Date: Tue Sep 30 18:07:15 2014 -0400
4000: fix copying pipeline instances with newer templates
* When copying script parameters from a pipeline instance, check that
the component exists in the source instance before blindly
dereferencing it.
* Added tests for copying when components=use_latest and when
script=use_same (the two edge cases exposed by this bug)
diff --git a/apps/workbench/app/controllers/pipeline_instances_controller.rb b/apps/workbench/app/controllers/pipeline_instances_controller.rb
index 8ee06c3..27e8f5b 100644
--- a/apps/workbench/app/controllers/pipeline_instances_controller.rb
+++ b/apps/workbench/app/controllers/pipeline_instances_controller.rb
@@ -16,12 +16,16 @@ class PipelineInstancesController < ApplicationController
@object.components.each do |cname, component|
# Go through the script parameters of each component
# that are marked as user input and copy them over.
- component[:script_parameters].each do |pname, val|
- if val.is_a? Hash and val[:dataclass]
- # this is user-inputtable, so check the value from the source pipeline
- srcvalue = source.components[cname][:script_parameters][pname]
- if not srcvalue.nil?
- component[:script_parameters][pname] = srcvalue
+ # Skip any components that are not present in the
+ # source instance (there's nothing to copy)
+ if source.components.include? cname
+ component[:script_parameters].each do |pname, val|
+ if val.is_a? Hash and val[:dataclass]
+ # this is user-inputtable, so check the value from the source pipeline
+ srcvalue = source.components[cname][:script_parameters][pname]
+ if not srcvalue.nil?
+ component[:script_parameters][pname] = srcvalue
+ end
end
end
end
@@ -33,7 +37,7 @@ class PipelineInstancesController < ApplicationController
if params['script'] == 'use_same'
# Go through each component and copy the script_version from each job.
@object.components.each do |cname, component|
- if source.components[cname][:job]
+ if source.components.include? cname and source.components[cname][:job]
component[:script_version] = source.components[cname][:job][:script_version]
end
end
diff --git a/apps/workbench/test/functional/pipeline_instances_controller_test.rb b/apps/workbench/test/functional/pipeline_instances_controller_test.rb
index 20886b5..69d0664 100644
--- a/apps/workbench/test/functional/pipeline_instances_controller_test.rb
+++ b/apps/workbench/test/functional/pipeline_instances_controller_test.rb
@@ -86,4 +86,42 @@ class PipelineInstancesControllerTest < ActionController::TestCase
assert assigns(:object).components[:foo][:job][:started_at].is_a? Time
assert assigns(:object).components[:foo][:job][:finished_at].is_a? Time
end
+
+ # The next two tests ensure that a pipeline instance can be copied
+ # when the template has components that do not exist in the
+ # instance (ticket #4000).
+
+ test "copy pipeline instance with components=use_latest" do
+ post(:copy,
+ {
+ id: api_fixture('pipeline_instances')['pipeline_with_newer_template']['uuid'],
+ components: 'use_latest',
+ script: 'use_latest',
+ pipeline_instance: {
+ state: 'RunningOnServer'
+ }
+ },
+ session_for(:active))
+ assert_response 302
+ assert_not_nil assigns(:object)
+ assert_not_nil assigns(:object).components[:foo]
+ assert_not_nil assigns(:object).components[:bar]
+ end
+
+ test "copy pipeline instance on newer template works with script=use_same" do
+ post(:copy,
+ {
+ id: api_fixture('pipeline_instances')['pipeline_with_newer_template']['uuid'],
+ components: 'use_latest',
+ script: 'use_same',
+ pipeline_instance: {
+ state: 'RunningOnServer'
+ }
+ },
+ session_for(:active))
+ assert_response 302
+ assert_not_nil assigns(:object)
+ assert_not_nil assigns(:object).components[:foo]
+ assert_not_nil assigns(:object).components[:bar]
+ end
end
diff --git a/services/api/test/fixtures/pipeline_instances.yml b/services/api/test/fixtures/pipeline_instances.yml
index d405c5f..2ccfab7 100644
--- a/services/api/test/fixtures/pipeline_instances.yml
+++ b/services/api/test/fixtures/pipeline_instances.yml
@@ -127,3 +127,16 @@ pipeline_to_merge_params:
default: [1,1,2,3,5]
array_with_value:
value: [1,1,2,3,5]
+
+pipeline_with_newer_template:
+ state: Complete
+ uuid: zzzzz-d1hrv-9fm8l10i9z2kqc6
+ owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ pipeline_template_uuid: zzzzz-p5p6p-vq4wuvy84xvaq2r
+ created_at: 2014-09-15 12:00:00
+ components:
+ foo:
+ script: foo
+ script_version: master
+ script_parameters:
+ input: foo_input
diff --git a/services/api/test/fixtures/pipeline_templates.yml b/services/api/test/fixtures/pipeline_templates.yml
index e56fcfa..bb825cf 100644
--- a/services/api/test/fixtures/pipeline_templates.yml
+++ b/services/api/test/fixtures/pipeline_templates.yml
@@ -78,3 +78,26 @@ parameter_with_search:
title: "Foo/bar pair"
description: "Provide a collection containing at least two files."
search_for: sometime # Matches baz_collection_in_asubproject
+
+new_pipeline_template:
+ # This template must include components that are not
+ # present in the pipeline instance 'pipeline_with_newer_template',
+ # at least one of which has a script_parameter that is a hash
+ # with a 'dataclass' field (ticket #4000)
+ uuid: zzzzz-p5p6p-vq4wuvy84xvaq2r
+ owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ created_at: 2014-09-14 12:00:00
+ modified_at: 2014-09-16 12:00:00
+ components:
+ foo:
+ script: foo
+ script_version: master
+ script_parameters: {}
+ bar:
+ script: bar
+ script_version: master
+ script_parameters:
+ input:
+ required: true
+ dataclass: Collection
+ title: "bar input"
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list