From ce8d052472be0cb417596c41f2700cf92a260fd3 Mon Sep 17 00:00:00 2001 From: Brett Smith Date: Thu, 24 Jul 2014 16:18:35 -0400 Subject: [PATCH] 3321: Workbench copes when pipeline components have an odd form. Closes #3321. --- .../pipeline_instances_controller.rb | 3 +- .../pipeline_templates_controller.rb | 3 +- .../app/helpers/pipeline_components_helper.rb | 13 +++ .../app/helpers/pipeline_instances_helper.rb | 4 + .../application/_pipeline_progress.html.erb | 2 +- .../_pipeline_status_label.html.erb | 2 +- .../_show_components.html.erb | 79 +------------------ .../_show_components_json.html.erb | 16 ++++ .../_show_components_running.html.erb | 67 ++++++++++++++++ .../pipeline_instances/_show_recent.html.erb | 4 +- .../_show_components.html.erb | 4 +- .../pipeline_instances_controller_test.rb | 7 ++ .../pipeline_templates_controller_test.rb | 6 ++ .../integration/pipeline_instances_test.rb | 12 ++- .../api/test/fixtures/pipeline_instances.yml | 22 +++++- .../api/test/fixtures/pipeline_templates.yml | 21 +++++ 16 files changed, 179 insertions(+), 86 deletions(-) create mode 100644 apps/workbench/app/helpers/pipeline_components_helper.rb create mode 100644 apps/workbench/app/views/pipeline_instances/_show_components_json.html.erb create mode 100644 apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb diff --git a/apps/workbench/app/controllers/pipeline_instances_controller.rb b/apps/workbench/app/controllers/pipeline_instances_controller.rb index e84d0e4597..b5c9815ed5 100644 --- a/apps/workbench/app/controllers/pipeline_instances_controller.rb +++ b/apps/workbench/app/controllers/pipeline_instances_controller.rb @@ -2,6 +2,7 @@ class PipelineInstancesController < ApplicationController skip_before_filter :find_object_by_uuid, only: :compare before_filter :find_objects_by_uuid, only: :compare include PipelineInstancesHelper + include PipelineComponentsHelper def copy @object = @object.dup @@ -196,7 +197,7 @@ class PipelineInstancesController < ApplicationController if @object and @object.state.in? ['New', 'Ready'] panes = %w(Inputs) + panes end - if not @object.components.values.collect { |x| x[:job] }.compact.any? + if not @object.components.values.any? { |x| x[:job] rescue false } panes -= ['Graph'] end panes diff --git a/apps/workbench/app/controllers/pipeline_templates_controller.rb b/apps/workbench/app/controllers/pipeline_templates_controller.rb index 2be51c6a18..2b2e9a4e33 100644 --- a/apps/workbench/app/controllers/pipeline_templates_controller.rb +++ b/apps/workbench/app/controllers/pipeline_templates_controller.rb @@ -1,5 +1,6 @@ class PipelineTemplatesController < ApplicationController - + include PipelineComponentsHelper + def show @objects = PipelineInstance.where(pipeline_template_uuid: @object.uuid) super diff --git a/apps/workbench/app/helpers/pipeline_components_helper.rb b/apps/workbench/app/helpers/pipeline_components_helper.rb new file mode 100644 index 0000000000..de951b3a97 --- /dev/null +++ b/apps/workbench/app/helpers/pipeline_components_helper.rb @@ -0,0 +1,13 @@ +module PipelineComponentsHelper + def render_pipeline_components(template_suffix, fallback=nil, locals={}) + begin + render(partial: "pipeline_instances/show_components_#{template_suffix}", + locals: locals) + rescue + case fallback + when :json + render(partial: "pipeline_instances/show_components_json") + end + end + end +end diff --git a/apps/workbench/app/helpers/pipeline_instances_helper.rb b/apps/workbench/app/helpers/pipeline_instances_helper.rb index 35d28d542b..c15c94cea8 100644 --- a/apps/workbench/app/helpers/pipeline_instances_helper.rb +++ b/apps/workbench/app/helpers/pipeline_instances_helper.rb @@ -52,6 +52,10 @@ module PipelineInstancesHelper object.components.each do |cname, c| i += 1 pj = {index: i, name: cname} + if not c.is_a?(Hash) + ret << pj + next + end pj[:job] = c[:job].is_a?(Hash) ? c[:job] : {} pj[:percent_done] = 0 pj[:percent_running] = 0 diff --git a/apps/workbench/app/views/application/_pipeline_progress.html.erb b/apps/workbench/app/views/application/_pipeline_progress.html.erb index d478f65ddc..2ae03a00aa 100644 --- a/apps/workbench/app/views/application/_pipeline_progress.html.erb +++ b/apps/workbench/app/views/application/_pipeline_progress.html.erb @@ -1,7 +1,7 @@ <% component_frac = 1.0 / p.components.length %>
<% p.components.each do |k,c| %> - <% if c[:job] %> + <% if c.is_a?(Hash) and c[:job] %> <%= render partial: "job_progress", locals: {:j => c[:job], :scaleby => component_frac } %> <% end %> <% end %> diff --git a/apps/workbench/app/views/application/_pipeline_status_label.html.erb b/apps/workbench/app/views/application/_pipeline_status_label.html.erb index f68d547aa5..8abf65b453 100644 --- a/apps/workbench/app/views/application/_pipeline_status_label.html.erb +++ b/apps/workbench/app/views/application/_pipeline_status_label.html.erb @@ -5,7 +5,7 @@ <% elsif p.state == 'RunningOnServer' || p.state == 'RunningOnClient' %> running <% else %> - <% if (p.components.select do |k,v| v[:job] end).length == 0 %> + <% if not p.components.values.any? { |c| c[:job] rescue false } %> not started <% else %> not running diff --git a/apps/workbench/app/views/pipeline_instances/_show_components.html.erb b/apps/workbench/app/views/pipeline_instances/_show_components.html.erb index 385001998d..c55a7253b8 100644 --- a/apps/workbench/app/views/pipeline_instances/_show_components.html.erb +++ b/apps/workbench/app/views/pipeline_instances/_show_components.html.erb @@ -6,73 +6,7 @@ Current state: <%= @object.state.sub('OnServer', '') %> 
- - - - - - - - - - - - - - - - - <% render_pipeline_jobs.each do |pj| %> - <% if pj[:job].andand[:uuid] - pipeline_job_uuids << pj[:job][:uuid] - end %> - - - <% current_job = Job.find(pj[:job][:uuid]) rescue nil %> - - - <% end %> - - - - -
- component - - job - <%# format:'js' here helps browsers avoid using the cached js - content in html context (e.g., duplicate tab -> see - javascript) %> - <%= link_to '(refresh)', {format: :js}, {class: 'refresh hide', remote: true, method: 'get'} %> -
- <%= pj[:name] %> - - <%= pj[:script] %> -
<%= pj[:script_version] %> - 		- <%= render(partial: 'job_status_label', locals: { j: pj[:job] }) %> - - <%= pj[:progress_bar] %> - - <% if current_job %> - <%= render partial: 'show_object_button', locals: {object: current_job, size: 'xs', link_text: 'Show job details'} %> - <% end %> - - <% if current_job.andand[:log] %> - <% fixup = /([a-f0-9]{32}\+\d+)(\+?.*)/.match(current_job[:log])%> - <% Collection.limit(1).where(uuid: fixup[1]).each do |c| %> - <% c.files.first.andand do |file| %> - <%= link_to url_for(controller: 'collections', action: 'show_file', uuid: current_job[:log], file: "#{file[0]}/#{file[1]}", disposition: 'inline', size: file[2]), class: 'btn btn-default btn-xs' do %> - Show log messages - <% end %> - <% end %> - <% end %> - <% end %> - - <% if current_job.andand[:output] %> - <%= link_to_if_arvados_object current_job[:output], {thumbnail: true, link_text: raw(' Show output files')}, {class: 'btn btn-default btn-xs'} %> - <% end %> -
+ <%= render_pipeline_components("running", :json, pipeline_job_uuids: pipeline_job_uuids) %> <% if @object.state.in? %w(RunningOnServer RunningOnClient Failed) %> @@ -88,14 +22,7 @@ <% else %> <%# state is either New or Ready %> +

Here are all of the pipeline's components (jobs that will need to run in order to complete the pipeline). If you know what you're doing (or you're experimenting) you can modify these parameters before starting the pipeline. Usually, you only need to edit the settings presented on the "Inputs" tab above.

- <% if @object.state.in? %w(New Ready) %> -

Here are all of the pipeline's components (jobs that will need to run in order to complete the pipeline). If you know what you're doing (or you're experimenting) you can modify these parameters before starting the pipeline. Usually, you only need to edit the settings presented on the "Inputs" tab above.

- <% end %> - - <% if @object.state.in? ['New', 'Ready'] %> - <%= render partial: 'show_components_editable', locals: {editable: true} %> - <% else %> - <%= render partial: 'show_components_editable', locals: {editable: false} %> - <% end %> + <%= render_pipeline_components("editable", :json, editable: true) %> <% end %> diff --git a/apps/workbench/app/views/pipeline_instances/_show_components_json.html.erb b/apps/workbench/app/views/pipeline_instances/_show_components_json.html.erb new file mode 100644 index 0000000000..3cdd5aedb3 --- /dev/null +++ b/apps/workbench/app/views/pipeline_instances/_show_components_json.html.erb @@ -0,0 +1,16 @@ +

The components of this pipeline are in a format that Workbench does not recognize.

+ +
+
+

+ + Show components JSON + +

+
+
+
+ 
<%= Oj.dump(@object.components, indent: 2) %>
+
+
+
diff --git a/apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb b/apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb new file mode 100644 index 0000000000..a41fdd1e2c --- /dev/null +++ b/apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb @@ -0,0 +1,67 @@ + + + + + + + + + + + + + + + + + <% render_pipeline_jobs.each do |pj| %> + <% if pj[:job].andand[:uuid] + pipeline_job_uuids << pj[:job][:uuid] + end %> + + + <% current_job = Job.find(pj[:job][:uuid]) rescue nil %> + + + <% end %> + + + + +
+ component + + job + <%# format:'js' here helps browsers avoid using the cached js + content in html context (e.g., duplicate tab -> see + javascript) %> + <%= link_to '(refresh)', {format: :js}, {class: 'refresh hide', remote: true, method: 'get'} %> +
+ <%= pj[:name] %> + + <%= pj[:script] %> +
<%= pj[:script_version] %> + 		+ <%= render(partial: 'job_status_label', locals: { j: pj[:job] }) %> + + <%= pj[:progress_bar] %> + + <% if current_job %> + <%= render partial: 'show_object_button', locals: {object: current_job, size: 'xs', link_text: 'Show job details'} %> + <% end %> + + <% if current_job.andand[:log] %> + <% fixup = /([a-f0-9]{32}\+\d+)(\+?.*)/.match(current_job[:log])%> + <% Collection.limit(1).where(uuid: fixup[1]).each do |c| %> + <% c.files.first.andand do |file| %> + <%= link_to url_for(controller: 'collections', action: 'show_file', uuid: current_job[:log], file: "#{file[0]}/#{file[1]}", disposition: 'inline', size: file[2]), class: 'btn btn-default btn-xs' do %> + Show log messages + <% end %> + <% end %> + <% end %> + <% end %> + + <% if current_job.andand[:output] %> + <%= link_to_if_arvados_object current_job[:output], {thumbnail: true, link_text: raw(' Show output files')}, {class: 'btn btn-default btn-xs'} %> + <% end %> +
diff --git a/apps/workbench/app/views/pipeline_instances/_show_recent.html.erb b/apps/workbench/app/views/pipeline_instances/_show_recent.html.erb index 0046b56f73..0b78e07d65 100644 --- a/apps/workbench/app/views/pipeline_instances/_show_recent.html.erb +++ b/apps/workbench/app/views/pipeline_instances/_show_recent.html.erb @@ -62,10 +62,10 @@ <% ob.components.each do |cname, c| %> - <% if c[:job] %> + <% if c.is_a?(Hash) and c[:job] %> <%= render partial: "job_status_label", locals: {:j => c[:job], :title => cname.to_s } %> <% else %> - <%= cname.to_s %> + <%= cname.to_s %> <% end %> <% end %> diff --git a/apps/workbench/app/views/pipeline_templates/_show_components.html.erb b/apps/workbench/app/views/pipeline_templates/_show_components.html.erb index bd48700003..1f2c1baaf4 100644 --- a/apps/workbench/app/views/pipeline_templates/_show_components.html.erb +++ b/apps/workbench/app/views/pipeline_templates/_show_components.html.erb @@ -11,8 +11,8 @@ }.to_json), { class: "btn btn-primary btn-sm", remote: true, method: 'get' } ) do %> - Run this pipeline + Run this pipeline <% end %> <% end %> -<%= render partial: 'pipeline_instances/show_components_editable', locals: {editable: false} %> +<%= render_pipeline_components("editable", :json, editable: false) %> diff --git a/apps/workbench/test/functional/pipeline_instances_controller_test.rb b/apps/workbench/test/functional/pipeline_instances_controller_test.rb index c04b5b1f90..fbfbf34e44 100644 --- a/apps/workbench/test/functional/pipeline_instances_controller_test.rb +++ b/apps/workbench/test/functional/pipeline_instances_controller_test.rb @@ -71,4 +71,11 @@ class PipelineInstancesControllerTest < ActionController::TestCase end end end + + test "component rendering copes with unexpeceted components format" do + get(:show, + {id: api_fixture("pipeline_instances")["components_is_jobspec"]["uuid"]}, + session_for(:active)) + assert_response :success + end end diff --git a/apps/workbench/test/functional/pipeline_templates_controller_test.rb b/apps/workbench/test/functional/pipeline_templates_controller_test.rb index be48e0ca93..82c4faeabe 100644 --- a/apps/workbench/test/functional/pipeline_templates_controller_test.rb +++ b/apps/workbench/test/functional/pipeline_templates_controller_test.rb @@ -1,4 +1,10 @@ require 'test_helper' class PipelineTemplatesControllerTest < ActionController::TestCase + test "component rendering copes with unexpeceted components format" do + get(:show, + {id: api_fixture("pipeline_templates")["components_is_jobspec"]["uuid"]}, + session_for(:active)) + assert_response :success + end end diff --git a/apps/workbench/test/integration/pipeline_instances_test.rb b/apps/workbench/test/integration/pipeline_instances_test.rb index 7053d39bfd..f5d6e633f3 100644 --- a/apps/workbench/test/integration/pipeline_instances_test.rb +++ b/apps/workbench/test/integration/pipeline_instances_test.rb @@ -103,7 +103,7 @@ class PipelineInstancesTest < ActionDispatch::IntegrationTest find('.btn', text: 'Run a pipeline').click within('.modal-dialog') do assert page.has_text? 'Two Part Pipeline Template' - find('.fa-gear').click + find('.selectable', text: 'Two Part Pipeline Template').click find('.btn', text: 'Next: choose inputs').click end @@ -146,4 +146,14 @@ class PipelineInstancesTest < ActionDispatch::IntegrationTest assert page.has_text? 'script_version' end + test "JSON popup available for strange components" do + uuid = api_fixture("pipeline_instances")["components_is_jobspec"]["uuid"] + visit page_with_token("active", "/pipeline_instances/#{uuid}") + click_on "Components" + assert(page.has_no_text?("script_parameters"), + "components JSON visible without popup") + click_on "Show components JSON" + assert(page.has_text?("script_parameters"), + "components JSON not found") + end end diff --git a/services/api/test/fixtures/pipeline_instances.yml b/services/api/test/fixtures/pipeline_instances.yml index 823116fda4..f73558df40 100644 --- a/services/api/test/fixtures/pipeline_instances.yml +++ b/services/api/test/fixtures/pipeline_instances.yml @@ -27,7 +27,6 @@ has_job: state: Ready uuid: zzzzz-d1hrv-1yfj6xkidf2muk3 owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz - owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz components: foo: script: foo @@ -37,3 +36,24 @@ has_job: uuid: zzzzz-8i9sb-pshmckwoma9plh7, script_version: master } + +components_is_jobspec: + # Helps test that clients cope with funny-shaped components. + # For an example, see #3321. + uuid: zzzzz-d1hrv-jobspeccomponts + owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz + created_at: 2014-04-14 12:35:04 -0400 + updated_at: 2014-04-14 12:35:04 -0400 + modified_at: 2014-04-14 12:35:04 -0400 + modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr + modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz + state: RunningOnServer + components: + script: foo + script_version: master + script_parameters: + input: + required: true + dataclass: Collection + title: "Foo/bar pair" + description: "Provide a collection containing at least two files." diff --git a/services/api/test/fixtures/pipeline_templates.yml b/services/api/test/fixtures/pipeline_templates.yml index 8e3a070ee6..012cd99230 100644 --- a/services/api/test/fixtures/pipeline_templates.yml +++ b/services/api/test/fixtures/pipeline_templates.yml @@ -36,3 +36,24 @@ two_part: default: [1,1,2,3,5] array_with_value: # important to test repeating values in the array! value: [1,1,2,3,5] + +components_is_jobspec: + # Helps test that clients cope with funny-shaped components. + # For an example, see #3321. + uuid: zzzzz-p5p6p-jobspeccomponts + owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz + created_at: 2014-04-14 12:35:04 -0400 + updated_at: 2014-04-14 12:35:04 -0400 + modified_at: 2014-04-14 12:35:04 -0400 + modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr + modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz + name: Pipeline Template with Jobspec Components + components: + script: foo + script_version: master + script_parameters: + input: + required: true + dataclass: Collection + title: "Foo/bar pair" + description: "Provide a collection containing at least two files." -- 2.30.2