[ARVADOS] updated: 1.2.0-16-g21810ed18

Git user git at public.curoverse.com
Wed Sep 12 08:14:59 EDT 2018


Summary of changes:
 .../app/views/projects/_show_dashboard.html.erb    |  4 +--
 .../app/views/work_units/_show_component.html.erb  | 40 +++++++++++++++-------
 doc/_includes/_container_runtime_status.liquid     | 18 ----------
 doc/api/methods/containers.html.textile.liquid     | 17 +++++++--
 sdk/cwl/arvados_cwl/__init__.py                    | 17 +++++----
 sdk/cwl/arvados_cwl/arvcontainer.py                |  6 +---
 sdk/cwl/arvados_cwl/done.py                        |  7 ++--
 services/api/app/models/container.rb               | 12 +++++++
 services/api/test/unit/container_test.rb           | 36 +++++++++++++++++++
 9 files changed, 105 insertions(+), 52 deletions(-)
 delete mode 100644 doc/_includes/_container_runtime_status.liquid

       via  21810ed18611adbb902bc7dc35407fc4e4adc828 (commit)
       via  c25c456973ebd634256e8c5eff56397d3381fb0e (commit)
       via  c5f1bd5d798400858a0314a30230fa8d860d4e01 (commit)
       via  d23b72550d31b105dfed14b8dc1dacaa1a71dd14 (commit)
      from  01693774dffd5ad5dd5313f24a5933f44b0e069d (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 21810ed18611adbb902bc7dc35407fc4e4adc828
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date:   Wed Sep 12 09:13:35 2018 -0300

    13773: Add well known runtime_status' keys data type validation.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 798124247..3f261eb6c 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -29,6 +29,7 @@ class Container < ArvadosModel
   before_validation :set_timestamps
   validates :command, :container_image, :output_path, :cwd, :priority, { presence: true }
   validates :priority, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
+  validate :validate_runtime_status
   validate :validate_state_change
   validate :validate_change
   validate :validate_lock
@@ -405,6 +406,17 @@ class Container < ArvadosModel
     end
   end
 
+  # Check that well-known runtime status keys have desired data types
+  def validate_runtime_status
+    [
+      'error', 'errorDetail', 'warning', 'warningDetail', 'activity'
+    ].each do |k|
+      if self.runtime_status.andand.include?(k) && !self.runtime_status[k].is_a?(String)
+        errors.add(:runtime_status, "'#{k}' value must be a string")
+      end
+    end
+  end
+
   def validate_change
     permitted = [:state]
 
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index c0bf4059d..b8acd4fd0 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -131,6 +131,42 @@ class ContainerTest < ActiveSupport::TestCase
     end
   end
 
+  test "Container runtime_status data types" do
+    set_user_from_auth :active
+    attrs = {
+      environment: {},
+      mounts: {"BAR" => {"kind" => "FOO"}},
+      output_path: "/tmp",
+      priority: 1,
+      runtime_constraints: {"vcpus" => 1, "ram" => 1}
+    }
+    c, _ = minimal_new(attrs)
+    assert_equal c.runtime_status, {}
+    assert_equal Container::Queued, c.state
+
+    set_user_from_auth :dispatch1
+    c.update_attributes! state: Container::Locked
+    c.update_attributes! state: Container::Running
+
+    [
+      'error', 'errorDetail', 'warning', 'warningDetail', 'activity'
+    ].each do |k|
+      # String type is allowed
+      string_val = 'A string is accepted'
+      c.update_attributes! runtime_status: {k => string_val}
+      assert_equal string_val, c.runtime_status[k]
+
+      # Other types aren't allowed
+      [
+        42, false, [], {}, nil
+      ].each do |unallowed_val|
+        assert_raises ActiveRecord::RecordInvalid do
+          c.update_attributes! runtime_status: {k => unallowed_val}
+        end
+      end
+    end
+  end
+
   test "Container runtime_status updates" do
     set_user_from_auth :active
     attrs = {

commit c25c456973ebd634256e8c5eff56397d3381fb0e
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date:   Tue Sep 11 11:44:48 2018 -0300

    13773: Enhance error/warning runtime status display on workbench.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>

diff --git a/apps/workbench/app/views/projects/_show_dashboard.html.erb b/apps/workbench/app/views/projects/_show_dashboard.html.erb
index cc00e0a8e..69abf04e6 100644
--- a/apps/workbench/app/views/projects/_show_dashboard.html.erb
+++ b/apps/workbench/app/views/projects/_show_dashboard.html.erb
@@ -100,7 +100,7 @@ SPDX-License-Identifier: AGPL-3.0 %>
             end
             %>
             <% if wu.is_finished? %>
-            <div class="dashboard-panel-info-row row-<%=wu.uuid%>" title="<%=runtime_status_tooltip%>">
+            <div class="dashboard-panel-info-row row-<%=wu.uuid%>" title="<%=sanitize(runtime_status_tooltip)%>">
               <div class="row">
                 <div class="col-md-6 text-overflow-ellipsis">
                   <%= link_to_if_arvados_object p, {friendly_name: true} %>
@@ -125,7 +125,7 @@ SPDX-License-Identifier: AGPL-3.0 %>
 
             </div>
             <% else %>
-            <div class="dashboard-panel-info-row row-<%=wu.uuid%>" title="<%=runtime_status_tooltip%>">
+            <div class="dashboard-panel-info-row row-<%=wu.uuid%>" title="<%=sanitize(runtime_status_tooltip)%>">
               <div class="row">
                 <div class="col-md-6 text-overflow-ellipsis">
                   <%= link_to_if_arvados_object p, {friendly_name: true} %>
diff --git a/apps/workbench/app/views/work_units/_show_component.html.erb b/apps/workbench/app/views/work_units/_show_component.html.erb
index b635b0044..cac263d1e 100644
--- a/apps/workbench/app/views/work_units/_show_component.html.erb
+++ b/apps/workbench/app/views/work_units/_show_component.html.erb
@@ -42,29 +42,43 @@ SPDX-License-Identifier: AGPL-3.0 %>
 <div class="container">
   <div class="col-md-12">
     <div class="panel panel-danger">
-      <div class="panel-heading">Error Information</div>
-      <div class="panel-body">
-        <%= wu.runtime_status[:error] %>
-        <%# Show collapsable detailed error information, if any %>
+      <div class="panel-heading">
+        <h4 class="panel-title">
+          <a class="component-detail-panel" data-toggle="collapse" href="#errorDetail">
+            <span class="caret"></span> Error: <%= sanitize(wu.runtime_status[:error]) %>
+          </a>
+        </h4>
+      </div>
+      <div id="errorDetail" class="panel-body panel-collapse collapse">
         <% if wu.runtime_status[:errorDetail] %>
-        <a class="btn btn-sm btn-primary pull-right" data-toggle="collapse" data-target="#errorDetail">Toggle details</a>
-        <div class="clearfix"></div>
-        <div id="errorDetail" class="collapse">
-          <pre><%= wu.runtime_status[:errorDetail] %></pre>
-        </div>
+          <pre><%= sanitize(wu.runtime_status[:errorDetail]) %></pre>
+        <% else %>
+          No detailed information available.
         <% end %>
       </div>
     </div>
   </div>
 </div>
+<% end %>
+
 <%# Display runtime warning message %>
-<% elsif wu.runtime_status.andand[:warning] %>
+<% if wu.runtime_status.andand[:warning] %>
 <div class="container">
   <div class="col-md-12">
     <div class="panel panel-warning">
-      <div class="panel-heading">Warning</div>
-      <div class="panel-body">
-        <%= wu.runtime_status[:warning] %>
+      <div class="panel-heading">
+        <h4 class="panel-title">
+          <a class="component-detail-panel" data-toggle="collapse" href="#warningDetail">
+            <span class="caret"></span> Warning: <%= sanitize(wu.runtime_status[:warning]) %>
+          </a>
+        </h4>
+      </div>
+      <div id="warningDetail" class="panel-body panel-collapse collapse">
+        <% if wu.runtime_status[:warningDetail] %>
+          <pre><%= sanitize(wu.runtime_status[:warningDetail]) %></pre>
+        <% else %>
+          No detailed information available.
+        <% end %>
       </div>
     </div>
   </div>

commit c5f1bd5d798400858a0314a30230fa8d860d4e01
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date:   Tue Sep 11 11:42:37 2018 -0300

    13773: Don't intercept logger.info() calls for runtime status.
    
    * On multi line log messages, use the first as a status and the rest as detail.
    * Revert done.logtail() usage as it's no longer needed for failed childs
    reporting.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>

diff --git a/sdk/cwl/arvados_cwl/__init__.py b/sdk/cwl/arvados_cwl/__init__.py
index e1bbcc2a7..5ea402a66 100644
--- a/sdk/cwl/arvados_cwl/__init__.py
+++ b/sdk/cwl/arvados_cwl/__init__.py
@@ -81,16 +81,16 @@ class RuntimeStatusLoggingHandler(logging.Handler):
             kind = 'error'
         elif record.levelno == logging.WARNING:
             kind = 'warning'
-        elif record.levelno == logging.INFO:
-            kind = 'activity'
         if kind is not None:
             log_msg = record.getMessage()
             if '\n' in log_msg:
-                # If the logged message is multi-line, include it as a detail
+                # If the logged message is multi-line, use its first line as status
+                # and the rest as detail.
+                status, detail = log_msg.split('\n', 1)
                 self.runtime_status_update(
                     kind,
-                    "%s from %s (please see details)" % (kind, record.name),
-                    log_msg
+                    "%s: %s" % (record.name, status),
+                    detail
                 )
             else:
                 self.runtime_status_update(
@@ -250,9 +250,12 @@ http://doc.arvados.org/install/install-api-server.html#disable_api_methods
             if kind == 'error':
                 if not runtime_status.get('error'):
                     runtime_status.update({
-                        'error': message,
-                        'errorDetail': detail or "No error logs available"
+                        'error': message
                     })
+                    if detail is not None:
+                        runtime_status.update({
+                            'errorDetail': detail
+                        })
                 # Further errors are only mentioned as a count.
                 else:
                     # Get anything before an optional 'and N more' string.
diff --git a/sdk/cwl/arvados_cwl/arvcontainer.py b/sdk/cwl/arvados_cwl/arvcontainer.py
index 670f54352..9a0c53d86 100644
--- a/sdk/cwl/arvados_cwl/arvcontainer.py
+++ b/sdk/cwl/arvados_cwl/arvcontainer.py
@@ -321,13 +321,9 @@ class ArvadosContainer(JobBase):
                                                            keep_client=self.arvrunner.keep_client,
                                                            num_retries=self.arvrunner.num_retries)
                 label = self.arvrunner.label(self)
-                error_log = done.logtail(
+                done.logtail(
                     logc, logger.error,
                     "%s (%s) error log:" % (label, record["uuid"]), maxlen=40)
-                self.arvrunner.runtime_status_update(
-                    "error",
-                    "%s %s failed" % (label, record["uuid"]),
-                    error_log)
 
             if record["output_uuid"]:
                 if self.arvrunner.trash_intermediate or self.arvrunner.intermediate_output_ttl:
diff --git a/sdk/cwl/arvados_cwl/done.py b/sdk/cwl/arvados_cwl/done.py
index 7f3cb36de..6d46e79cb 100644
--- a/sdk/cwl/arvados_cwl/done.py
+++ b/sdk/cwl/arvados_cwl/done.py
@@ -57,8 +57,7 @@ timestamp_re = re.compile(r"^(\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d\.\d+Z) (.*)")
 
 def logtail(logcollection, logfunc, header, maxlen=25):
     if len(logcollection) == 0:
-        logfunc(header)
-        logfunc("  ** log is empty **")
+        logfunc("%s\n%s", header, "  ** log is empty **")
         return
 
     containersapi = ("crunch-run.txt" in logcollection)
@@ -95,6 +94,4 @@ def logtail(logcollection, logfunc, header, maxlen=25):
         loglines = mergelogs.values()[0]
 
     logtxt = "\n  ".join(l.strip() for l in loglines)
-    logfunc(header)
-    logfunc("\n  %s", logtxt)
-    return logtxt
+    logfunc("%s\n\n  %s", header, logtxt)

commit d23b72550d31b105dfed14b8dc1dacaa1a71dd14
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date:   Mon Sep 10 14:48:38 2018 -0300

    13773: Updates documentation
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>

diff --git a/doc/_includes/_container_runtime_status.liquid b/doc/_includes/_container_runtime_status.liquid
deleted file mode 100644
index 77051beb3..000000000
--- a/doc/_includes/_container_runtime_status.liquid
+++ /dev/null
@@ -1,18 +0,0 @@
-{% comment %}
-Copyright (C) The Arvados Authors. All rights reserved.
-
-SPDX-License-Identifier: CC-BY-SA-3.0
-{% endcomment %}
-
-h2. Runtime status
-
-Runtime status provides container's relevant information about its progress even while it's still in Running state. This is used to avoid reusing containers that have not yet failed but will definitely do, and also for easier workflow debugging.
-
-The following keys have well known meanings:
-
-table(table table-bordered table-condensed).
-|_. Key|_. Type|_. Description|_. Notes|
-|error|string|The existance of this key indicates the container will definitely fail, or has already failed.|Optional.|
-|warning|string|Indicates something unusual happened or is currently happening, but isn't considered fatal.|Optional.|
-|activity|string|A message for the end user about what state the container is currently in.|Optional.|
-|errorDetails|string|Additional structured error details.|Optional.|
diff --git a/doc/api/methods/containers.html.textile.liquid b/doc/api/methods/containers.html.textile.liquid
index 3384f9377..f13bf400a 100644
--- a/doc/api/methods/containers.html.textile.liquid
+++ b/doc/api/methods/containers.html.textile.liquid
@@ -43,7 +43,7 @@ Generally this will contain additional keys that are not present in any correspo
 }</code></pre>See "Runtime constraints":#runtime_constraints for more details.|
 |runtime_status|hash|Information related to the container's run, including its steps. Some keys have specific meaning and are described later in this page.|e.g.,
 <pre><code>{
-  "error": "This container won't be successful because at least one step have already failed."
+  "error": "This container won't be successful because at least one step has already failed."
 }</code></pre>See "Runtime status":#runtime_status for more details.|
 |scheduling_parameters|hash|Parameters to be passed to the container scheduler when running this container.|e.g.,<pre><code>{
 "partitions":["fastcpu","vfastcpu"]
@@ -70,7 +70,20 @@ h2(#mount_types). {% include 'mount_types' %}
 
 h2(#runtime_constraints). {% include 'container_runtime_constraints' %}
 
-h2(#runtime_status). {% include 'container_runtime_status' %}
+h2(#runtime_status). Runtime status
+
+Runtime status provides container's relevant information about its progress even while it's still in Running state. This is used to avoid reusing containers that have not yet failed but will definitely do, and also for easier workflow debugging.
+
+The following keys have well known meanings:
+
+table(table table-bordered table-condensed).
+|_. Key|_. Type|_. Description|_. Notes|
+|error|string|The existance of this key indicates the container will definitely fail, or has already failed.|Optional.|
+|warning|string|Indicates something unusual happened or is currently happening, but isn't considered fatal.|Optional.|
+|activity|string|A message for the end user about what state the container is currently in.|Optional.|
+|errorDetails|string|Additional structured error details.|Optional.|
+|warningDetails|string|Additional structured warning details.|Optional.|
+|activityDetails|string|Additional structured activity details.|Optional.|
 
 h2(#scheduling_parameters). {% include 'container_scheduling_parameters' %}
 

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list