[ARVADOS] updated: ce8d052472be0cb417596c41f2700cf92a260fd3

git at public.curoverse.com git at public.curoverse.com
Thu Jul 24 16:17:19 EDT 2014


Summary of changes:
 .../controllers/pipeline_instances_controller.rb   |  3 +-
 .../controllers/pipeline_templates_controller.rb   |  3 +-
 .../app/helpers/pipeline_components_helper.rb      | 13 ++++
 .../app/helpers/pipeline_instances_helper.rb       |  4 ++
 .../views/application/_pipeline_progress.html.erb  |  2 +-
 .../application/_pipeline_status_label.html.erb    |  2 +-
 .../pipeline_instances/_show_components.html.erb   | 79 +---------------------
 .../_show_components_json.html.erb                 | 16 +++++
 ....html.erb => _show_components_running.html.erb} | 34 ----------
 .../views/pipeline_instances/_show_recent.html.erb |  4 +-
 .../pipeline_templates/_show_components.html.erb   |  4 +-
 .../pipeline_instances_controller_test.rb          |  7 ++
 .../pipeline_templates_controller_test.rb          |  6 ++
 .../test/integration/pipeline_instances_test.rb    | 12 +++-
 services/api/test/fixtures/pipeline_instances.yml  | 22 +++++-
 services/api/test/fixtures/pipeline_templates.yml  | 21 ++++++
 16 files changed, 112 insertions(+), 120 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
 copy apps/workbench/app/views/pipeline_instances/{_show_components.html.erb => _show_components_running.html.erb} (63%)

       via  ce8d052472be0cb417596c41f2700cf92a260fd3 (commit)
      from  15c61d99c5a867c6a58ef3eb10cc72d7f9ec5cd8 (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 ce8d052472be0cb417596c41f2700cf92a260fd3
Author: Brett Smith <brett at curoverse.com>
Date:   Thu Jul 24 16:18:35 2014 -0400

    3321: Workbench copes when pipeline components have an odd form.
    
    Closes #3321.

diff --git a/apps/workbench/app/controllers/pipeline_instances_controller.rb b/apps/workbench/app/controllers/pipeline_instances_controller.rb
index e84d0e4..b5c9815 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 2be51c6..2b2e9a4 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 0000000..de951b3
--- /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 35d28d5..c15c94c 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 d478f65..2ae03a0 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 %>
 <div class="progress">
   <% 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 f68d547..8abf65b 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' %>
   <span class="label label-info">running</span>
 <% else %>
-  <% if (p.components.select do |k,v| v[:job] end).length == 0 %>
+  <% if not p.components.values.any? { |c| c[:job] rescue false } %>
     <span class="label label-default">not started</span>
   <% else %>
     <span class="label label-default">not running</span>
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 3850019..c55a725 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: <span class="badge badge-info" data-pipeline-state="<%= @object.state %>"><%= @object.state.sub('OnServer', '') %></span> 
   </div>
 
-  <table class="table pipeline-components-table">
-    <colgroup>
-      <col style="width: 15%" />
-      <col style="width: 25%" />
-      <col style="width: 8%" />
-      <col style="width: 13%" />
-      <col style="width: 12%" />
-      <col style="width: 14%" />
-      <col style="width: 13%" />
-    </colgroup>
-    <thead>
-      <tr>
-        <th colspan="2">
-          component
-        </th><th colspan="5">
-          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'} %>
-        </th>
-      </tr>
-    </thead>
-    <tbody>
-      <% render_pipeline_jobs.each do |pj| %>
-        <% if pj[:job].andand[:uuid]
-             pipeline_job_uuids << pj[:job][:uuid]
-           end %>
-      <tr>
-        <td>
-          <%= pj[:name] %>
-        </td><td>
-          <%= pj[:script] %>
-          <br /><span class="deemphasize"><%= pj[:script_version] %></span>
-        </td><td>
-          <%= render(partial: 'job_status_label', locals: { j: pj[:job] }) %>
-        </td><td>
-          <%= pj[:progress_bar] %>
-        </td>
-        <% current_job = Job.find(pj[:job][:uuid]) rescue nil %>
-        <td>
-          <% if current_job %>
-            <%= render partial: 'show_object_button', locals: {object: current_job, size: 'xs', link_text: 'Show job details'} %>
-          <% end %>
-        </td><td>
-          <% 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 %>
-                  <i class="fa fa-fw fa-info"></i> Show log messages
-                <% end %>
-              <% end %>
-            <% end %>
-          <% end %>
-        </td><td>
-          <% if current_job.andand[:output] %>
-            <%= link_to_if_arvados_object current_job[:output], {thumbnail: true, link_text: raw('<i class="fa fa-fw fa-archive"></i> Show output files')}, {class: 'btn btn-default btn-xs'} %>
-          <% end %>
-        </td>
-      </tr>
-      <% end %>
-    </tbody>
-    <tfoot>
-      <tr><td colspan="7"></td></tr>
-    </tfoot>
-  </table>
+  <%= 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 %>
+  <p><i>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.</i></p>
 
-  <% if @object.state.in? %w(New Ready) %>
-    <p><i>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.</i></p>
-  <% 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 0000000..3cdd5ae
--- /dev/null
+++ b/apps/workbench/app/views/pipeline_instances/_show_components_json.html.erb
@@ -0,0 +1,16 @@
+<p>The components of this pipeline are in a format that Workbench does not recognize.</p>
+
+    <div id="components-accordion" class="panel panel-default">
+      <div class="panel-heading">
+        <h4 class="panel-title">
+          <a data-toggle="collapse" data-parent="#components-accordion" href="#components-json">
+            Show components JSON
+          </a>
+        </h4>
+      </div>
+      <div id="components-json" class="panel-collapse collapse">
+        <div class="panel-body">
+          <pre><%= Oj.dump(@object.components, indent: 2) %></pre>
+        </div>
+      </div>
+    </div>
diff --git a/apps/workbench/app/views/pipeline_instances/_show_components.html.erb b/apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb
similarity index 63%
copy from apps/workbench/app/views/pipeline_instances/_show_components.html.erb
copy to apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb
index 3850019..a41fdd1 100644
--- a/apps/workbench/app/views/pipeline_instances/_show_components.html.erb
+++ b/apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb
@@ -1,11 +1,3 @@
-<% if !@object.state.in? ['New', 'Ready'] %>
-
-  <% pipeline_job_uuids = [] %>
-
-  <div class="pull-right">
-    Current state: <span class="badge badge-info" data-pipeline-state="<%= @object.state %>"><%= @object.state.sub('OnServer', '') %></span> 
-  </div>
-
   <table class="table pipeline-components-table">
     <colgroup>
       <col style="width: 15%" />
@@ -73,29 +65,3 @@
       <tr><td colspan="7"></td></tr>
     </tfoot>
   </table>
-
-  <% if @object.state.in? %w(RunningOnServer RunningOnClient Failed) %>
-
-      <h4>Log messages from jobs</h4>
-      <% log_history = pipeline_log_history((pipeline_job_uuids || []) + [@object.uuid]) %>
-      <div class="arv-log-event-listener arv-log-event-handler-append-logs arv-job-log-window" id="pipeline_event_log_div" data-object-uuids="<%= @object.uuid %> <%=(pipeline_job_uuids || []).join(" ")%>">
-        <% log_history.each do |entry| %>
-          <%=entry%><br/>
-        <% end %>
-      </div>
-
-  <% end %>
-
-<% else %>
-  <%# state is either New or Ready %>
-
-  <% if @object.state.in? %w(New Ready) %>
-    <p><i>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.</i></p>
-  <% 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 %>
-<% 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 0046b56..0b78e07 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 @@
       </td>
       <td style="border-top: 0; opacity: 0.5;" colspan="6">
         <% 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 %>
-            <span class="label label-default"><%= cname.to_s %></span>            
+            <span class="label label-default"><%= cname.to_s %></span>
           <% end %>
         <% end %>
       </td>
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 bd48700..1f2c1ba 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 c04b5b1..fbfbf34 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 be48e0c..82c4fae 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 7053d39..f5d6e63 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 823116f..f73558d 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 8e3a070..012cd99 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."

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list