From a0187bc7327e7abfc759a43cee81cb77fe063bf0 Mon Sep 17 00:00:00 2001 From: radhika Date: Mon, 30 May 2016 20:41:32 -0400 Subject: [PATCH] 8876: For jobs also, compute progress from it's children if present; otherwise, use task_summary. --- apps/workbench/app/models/job_work_unit.rb | 30 ++----- .../app/models/pipeline_instance_work_unit.rb | 31 +------ apps/workbench/app/models/proxy_work_unit.rb | 71 ++++++++++++++++ apps/workbench/app/models/work_unit.rb | 4 + .../app/views/work_unit/_show_child.html.erb | 13 +-- .../views/work_unit/_show_component.html.erb | 19 +++-- apps/workbench/test/integration/jobs_test.rb | 85 +++++++++++-------- apps/workbench/test/unit/work_unit_test.rb | 2 +- services/api/test/fixtures/links.yml | 15 ++++ 9 files changed, 166 insertions(+), 104 deletions(-) diff --git a/apps/workbench/app/models/job_work_unit.rb b/apps/workbench/app/models/job_work_unit.rb index 8fc3285857..256003795a 100644 --- a/apps/workbench/app/models/job_work_unit.rb +++ b/apps/workbench/app/models/job_work_unit.rb @@ -1,9 +1,9 @@ class JobWorkUnit < ProxyWorkUnit def children - uuid = get(:uuid) - items = [] + return self.my_children if self.my_children # Jobs components + items = [] components = get(:components) uuids = components.andand.collect {|_, v| v} return items if (!uuids or uuids.empty?) @@ -19,26 +19,8 @@ class JobWorkUnit < ProxyWorkUnit items << obj.work_unit(components.key(obj.uuid)) end end - items - end - - def progress - state = get(:state) - if state == 'Complete' - return 1.0 - end - tasks_summary = get(:tasks_summary) - failed = tasks_summary[:failed] || 0 rescue 0 - done = tasks_summary[:done] || 0 rescue 0 - running = tasks_summary[:running] || 0 rescue 0 - todo = tasks_summary[:todo] || 0 rescue 0 - if done + running + failed + todo > 0 - total_tasks = done + running + failed + todo - (done+failed).to_f / total_tasks - else - 0.0 - end + self.my_children = items end def docker_image @@ -62,7 +44,11 @@ class JobWorkUnit < ProxyWorkUnit end def child_summary - get(:tasks_summary) + if children.any? + super + else + get(:tasks_summary) + end end def can_cancel? diff --git a/apps/workbench/app/models/pipeline_instance_work_unit.rb b/apps/workbench/app/models/pipeline_instance_work_unit.rb index 578e193c6e..8285424a29 100644 --- a/apps/workbench/app/models/pipeline_instance_work_unit.rb +++ b/apps/workbench/app/models/pipeline_instance_work_unit.rb @@ -1,5 +1,7 @@ class PipelineInstanceWorkUnit < ProxyWorkUnit def children + return self.my_children if self.my_children + items = [] jobs = {} @@ -27,34 +29,7 @@ class PipelineInstanceWorkUnit < ProxyWorkUnit end end - items - end - - def progress - state = get(:state) - if state == 'Complete' - return 1.0 - end - - done = 0 - failed = 0 - todo = 0 - children.each do |c| - if c.success?.nil? - todo = todo+1 - elsif c.success? - done = done+1 - else - failed = failed+1 - end - end - - if done + failed + todo > 0 - total = done + failed + todo - (done+failed).to_f / total - else - 0.0 - end + self.my_children = items end def uri diff --git a/apps/workbench/app/models/proxy_work_unit.rb b/apps/workbench/app/models/proxy_work_unit.rb index 9798361806..b8680a6b50 100644 --- a/apps/workbench/app/models/proxy_work_unit.rb +++ b/apps/workbench/app/models/proxy_work_unit.rb @@ -3,6 +3,7 @@ class ProxyWorkUnit < WorkUnit attr_accessor :lbl attr_accessor :proxied + attr_accessor :my_children attr_accessor :unreadable_children def initialize proxied, label @@ -74,6 +75,76 @@ class ProxyWorkUnit < WorkUnit end end + def child_summary + done = 0 + failed = 0 + todo = 0 + running = 0 + children.each do |c| + case c.state_label + when 'Complete' + done = done+1 + when 'Failed', 'Cancelled' + failed = failed+1 + when 'Running' + running = running+1 + else + todo = todo+1 + end + end + + summary = {} + summary[:done] = done + summary[:failed] = failed + summary[:todo] = todo + summary[:running] = running + summary + end + + def child_summary_str + summary = child_summary + summary_txt = '' + + if state_label == 'Running' + if summary[:done] + summary_txt += "#{summary[:done]} #{'child'.pluralize(summary[:done])} done," + end + if summary[:failed] + summary_txt += "#{summary[:failed]} failed," + end + if summary[:running] + summary_txt += "#{summary[:running]} running," + end + if summary[:todo] + summary_txt += "#{summary[:todo]} pending" + end + end + summary_txt + end + + def progress + state = get(:state) + if state == 'Complete' + return 1.0 + elsif state == 'Failed' or state== 'Cancelled' + return 0.0 + end + + summary = child_summary + return 0.0 if summary.nil? + + done = summary[:done] || 0 + running = summary[:running] || 0 + failed = summary[:failed] || 0 + todo = summary[:todo] || 0 + total = done + running + failed + todo + if total > 0 + (done+failed).to_f / total + else + 0.0 + end + end + def parameters get(:script_parameters) end diff --git a/apps/workbench/app/models/work_unit.rb b/apps/workbench/app/models/work_unit.rb index 7439128ed7..fba9015d19 100644 --- a/apps/workbench/app/models/work_unit.rb +++ b/apps/workbench/app/models/work_unit.rb @@ -95,6 +95,10 @@ class WorkUnit # summary status of any children of this work unit end + def child_summary_str + # textual representation of child summary + end + def can_cancel? # returns true if this work unit can be canceled end diff --git a/apps/workbench/app/views/work_unit/_show_child.html.erb b/apps/workbench/app/views/work_unit/_show_child.html.erb index 0824871fb2..64881a1ff0 100644 --- a/apps/workbench/app/views/work_unit/_show_child.html.erb +++ b/apps/workbench/app/views/work_unit/_show_child.html.erb @@ -63,17 +63,8 @@ <%# column offset 8 %>
- <% if current_obj.child_summary[:done] %> - <%= current_obj.child_summary[:done] %> <%= "child".pluralize(current_obj.child_summary[:done]) %> done, - <% end %> - <% if current_obj.child_summary[:failed] %> - <%= current_obj.child_summary[:failed] %> failed, - <% end %> - <% if current_obj.child_summary[:running] %> - <%= current_obj.child_summary[:running] %> running, - <% end %> - <% if current_obj.child_summary[:todo] %> - <%= current_obj.child_summary[:todo] %> pending + <% if current_obj.child_summary_str %> + <%= current_obj.child_summary_str %> <% end %>
diff --git a/apps/workbench/app/views/work_unit/_show_component.html.erb b/apps/workbench/app/views/work_unit/_show_component.html.erb index 23ac3d393d..dbf1c11f8d 100644 --- a/apps/workbench/app/views/work_unit/_show_component.html.erb +++ b/apps/workbench/app/views/work_unit/_show_component.html.erb @@ -4,10 +4,18 @@
<%# Need additional handling for main object display %> <% if @object.uuid == wu.uuid %> -
+
+
-
- <%# column offset 0 %> +
+ <% if wu.state_label == 'Running' and wu.child_summary_str %> +
+ <%= wu.child_summary_str %> +
+ <% end %> +
+ <%= render partial: 'work_unit/progress', locals: {wu: wu} %> +
<% if wu.state_label.in? ["Queued", "Running"] and wu.can_cancel? and @object.editable? %>
<%= form_tag "#{wu.uri}/cancel", remote: true, style: "display:inline; padding-left: 1em" do |f| %> @@ -16,13 +24,10 @@ <% end %>
<% end %> - <%# column offset 1 %> -
- <%= render partial: 'work_unit/progress', locals: {wu: wu} %> -
+
<% end %>
diff --git a/apps/workbench/test/integration/jobs_test.rb b/apps/workbench/test/integration/jobs_test.rb index de04aa8c73..1ad3296071 100644 --- a/apps/workbench/test/integration/jobs_test.rb +++ b/apps/workbench/test/integration/jobs_test.rb @@ -127,44 +127,59 @@ class JobsTest < ActionDispatch::IntegrationTest end end - test 'view job with components' do - job = api_fixture('jobs')['running_job_with_components'] - component1 = api_fixture('jobs')['completed_job_in_publicly_accessible_project'] - component2 = api_fixture('pipeline_instances')['running_pipeline_with_complete_job'] - component2_child1 = api_fixture('jobs')['previous_job_run'] - component2_child2 = api_fixture('jobs')['running'] - - visit page_with_token("active", "/jobs/#{job['uuid']}") - assert page.has_text? job['script_version'] - assert page.has_no_text? 'script_parameters' + [ + ['active', true], + ['job_reader', false], + ].each do |user, readable| + test "view job with components as #{user} user" do + job = api_fixture('jobs')['running_job_with_components'] + component1 = api_fixture('jobs')['completed_job_in_publicly_accessible_project'] + component2 = api_fixture('pipeline_instances')['running_pipeline_with_complete_job'] + component2_child1 = api_fixture('jobs')['previous_job_run'] + component2_child2 = api_fixture('jobs')['running'] + + visit page_with_token(user, "/jobs/#{job['uuid']}") + assert page.has_text? job['script_version'] + assert page.has_no_text? 'script_parameters' + + if readable + assert page.has_link? 'component1' + assert page.has_link? 'component2' + else + # children are not readable by user + assert page.has_no_link? 'component1' + assert page.has_no_link? 'component2' + return + end - click_link('component1') - within('#collapse1') do - assert(has_text? component1['uuid']) - assert(has_text? component1['script_version']) - assert(has_text? 'script_parameters') - end - click_link('component1') - - click_link('component2') - within('#collapse2') do - assert(has_text? component2['uuid']) - assert(has_text? component2['script_version']) - assert(has_no_text? 'script_parameters') - assert(has_link? 'previous') - assert(has_link? 'running') - - click_link('previous') - within('#collapse3') do - assert(has_text? component2_child1['uuid']) - assert(has_text? component2_child1['script_version']) + click_link('component1') + within('#collapse1') do + assert(has_text? component1['uuid']) + assert(has_text? component1['script_version']) + assert(has_text? 'script_parameters') end - click_link('previous') + click_link('component1') + + click_link('component2') + within('#collapse2') do + assert(has_text? component2['uuid']) + assert(has_text? component2['script_version']) + assert(has_no_text? 'script_parameters') + assert(has_link? 'previous') + assert(has_link? 'running') + + click_link('previous') + within('#collapse3') do + assert(has_text? component2_child1['uuid']) + assert(has_text? component2_child1['script_version']) + end + click_link('previous') - click_link('running') - within('#collapse4') do - assert(has_text? component2_child2['uuid']) - assert(has_text? component2_child2['script_version']) + click_link('running') + within('#collapse4') do + assert(has_text? component2_child2['uuid']) + assert(has_text? component2_child2['script_version']) + end end end end diff --git a/apps/workbench/test/unit/work_unit_test.rb b/apps/workbench/test/unit/work_unit_test.rb index 63f6a5aacd..a1de629609 100644 --- a/apps/workbench/test/unit/work_unit_test.rb +++ b/apps/workbench/test/unit/work_unit_test.rb @@ -2,7 +2,7 @@ require 'test_helper' class WorkUnitTest < ActiveSupport::TestCase [ - [Job, 'running_job_with_components', "jwu", 2, "Running", nil, 0.2], + [Job, 'running_job_with_components', "jwu", 2, "Running", nil, 0.5], [PipelineInstance, 'pipeline_in_running_state', nil, 1, "Running", nil, 0.0], [PipelineInstance, 'has_component_with_completed_jobs', nil, 3, "Complete", true, 1.0], [PipelineInstance, 'pipeline_with_tagged_collection_input', "pwu", 1, "Ready", nil, 0.0], diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml index 7ed7f6bcf3..96e82ccbc9 100644 --- a/services/api/test/fixtures/links.yml +++ b/services/api/test/fixtures/links.yml @@ -702,6 +702,21 @@ job_reader_can_read_previous_job_run: tail_uuid: zzzzz-tpzed-905b42d1dd4a354 head_uuid: zzzzz-8i9sb-cjs4pklxxjykqqq +job_reader_can_read_job_with_components: + # Permission link giving job_reader permission + # to read running_job_with_components + uuid: zzzzz-o0j2j-jobcomps4jobrdr + owner_uuid: zzzzz-tpzed-000000000000000 + created_at: 2014-06-13 20:42:26 -0800 + modified_by_client_uuid: zzzzz-tpzed-000000000000000 + modified_by_user_uuid: zzzzz-tpzed-000000000000000 + modified_at: 2014-06-13 20:42:26 -0800 + updated_at: 2014-06-13 20:42:26 -0800 + link_class: permission + name: can_read + tail_uuid: zzzzz-tpzed-905b42d1dd4a354 + head_uuid: zzzzz-8i9sb-with2components + job_reader_can_read_foo_repo: # Permission link giving job_reader permission # to read foo_repo -- 2.30.2