Merge pull request #3467 from OSC/r_3.1.3

R 3.1.3

apps/dashboard/Gemfile

@@ -48,7 +48,7 @@ gem 'mocha', '~> 2.1', group: :test
 gem 'autoprefixer-rails', '~> 10.2.5'
 gem 'dotiw'
 gem 'local_time', '~> 1.0.3'
-gem 'zip_tricks', '~> 5.5'
+gem 'zip_kit', '~> 6.2'
 gem 'rss', '~> 0.2'
 
 gem 'jsbundling-rails', '~> 1.0'
@@ -57,7 +57,7 @@ gem 'cssbundling-rails', '~> 1.1'
 # OOD specific gems
 gem 'ood_support', '~> 0.0.2'
 gem 'ood_appkit', '~> 2.1.0'
-gem 'ood_core', '~> 0.24.1'
+gem 'ood_core', '~> 0.25'
 gem 'pbs', '~> 2.2.1'
 gem 'rest-client', '~> 2.0'
 

apps/dashboard/Gemfile.lock

@@ -168,7 +168,7 @@ GEM
       ood_core (~> 0.1)
       rails (>= 6.0.0, < 7)
       redcarpet (~> 3.2)
-    ood_core (0.24.2)
+    ood_core (0.25.0)
       ffi (~> 1.9, >= 1.9.6)
       ood_support (~> 0.0.2)
       rexml (~> 3.2)
@@ -214,7 +214,7 @@ GEM
       rake (>= 12.2)
       thor (~> 1.0)
     rake (13.1.0)
-    rdoc (6.6.2)
+    rdoc (6.6.3.1)
       psych (>= 4.0.0)
     redcarpet (3.6.0)
     regexp_parser (2.9.0)
@@ -270,7 +270,7 @@ GEM
     xpath (3.2.0)
       nokogiri (~> 1.8)
     zeitwerk (2.6.12)
-    zip_tricks (5.6.0)
+    zip_kit (6.2.2)
 
 PLATFORMS
   ruby
@@ -293,7 +293,7 @@ DEPENDENCIES
   local_time (~> 1.0.3)
   mocha (~> 2.1)
   ood_appkit (~> 2.1.0)
-  ood_core (~> 0.24.1)
+  ood_core (~> 0.25)
   ood_support (~> 0.0.2)
   pbs (~> 2.2.1)
   rails (= 6.1.7.6)
@@ -306,7 +306,7 @@ DEPENDENCIES
   sinatra-contrib
   timecop (~> 0.9)
   webrick
-  zip_tricks (~> 5.5)
+  zip_kit (~> 6.2)
 
 BUNDLED WITH
    2.3.6

apps/dashboard/app/controllers/files_controller.rb

@@ -1,6 +1,6 @@
 # The controller for all the files pages /dashboard/files
 class FilesController < ApplicationController
-  include ZipTricks::RailsStreaming
+  include ActionController::Live
 
   before_action :strip_sendfile_headers, only: [:fs]
 
@@ -58,22 +58,25 @@ def fs
             response.sending_file = true
             response.cache_control[:public] ||= false
 
-            # FIXME: strategy 1: is below, use zip_tricks
-            # strategy 2: use actual zip command (likely much faster) and ActionController::Live
-            zip_tricks_stream do |zip|
-              @path.files_to_zip.each do |file|
-                begin
-                  next unless File.readable?(file.realpath)
-
-                  if File.file?(file.realpath)
-                    zip.write_deflated_file(file.relative_path.to_s) do |zip_file|
-                      IO.copy_stream(file.realpath, zip_file)
+            zip_headers = ZipKit::OutputEnumerator.new.streaming_http_headers
+            response.headers.merge!(zip_headers)
+
+            send_stream(filename: zipname) do |stream|
+              ZipKit::Streamer.open(stream) do |zip|
+                @path.files_to_zip.each do |file|
+                  begin
+                    next unless File.readable?(file.realpath)
+
+                    if File.file?(file.realpath)
+                      zip.write_deflated_file(file.relative_path.to_s) do |zip_file|
+                        IO.copy_stream(file.realpath, zip_file)
+                      end
+                    else
+                      zip.add_empty_directory(dirname: file.relative_path.to_s)
                     end
-                  else
-                    zip.add_empty_directory(dirname: file.relative_path.to_s)
+                  rescue => e
+                    logger.warn("error writing file #{file.path} to zip: #{e.message}")
                   end
-                rescue => e
-                  logger.warn("error writing file #{file.path} to zip: #{e.message}")
                 end
               end
             end
@@ -178,6 +181,20 @@ def edit
 
   private
 
+  def send_stream(filename:, disposition: "attachment", type: nil)
+    response.headers["Content-Type"] =
+      (type.is_a?(Symbol) ? Mime[type].to_s : type) ||
+      Mime::Type.lookup_by_extension(File.extname(filename).downcase.delete("."))&.to_s ||
+      "application/octet-stream"
+
+    response.headers["Content-Disposition"] =
+      ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: filename)
+
+    yield response.stream
+  ensure
+    response.stream.close
+  end
+
   # set these headers to nil so that we (Rails) will read files
   # off of disk instead of nginx.
   def strip_sendfile_headers

apps/dashboard/app/javascript/dynamic_forms.js

@@ -418,7 +418,6 @@ function updateVisibility(event, changeId) {
   $(`#${changeId}`).parents().each(function(_i, parent) {
     if(parent.classList.contains('form-group')) {
       changeElement = $(parent);
-      return false;
     }
   });
 

apps/dashboard/app/models/batch_connect/session.rb

@@ -31,6 +31,10 @@ def get_binding
     # @return [Integer] created at
     attr_accessor :created_at
 
+    # When this session finished, as a unix timestamp
+    # @return [Integer] completed at
+    attr_accessor :completed_at
+
     # Token describing app and sub app
     # @return [String] app token
     attr_accessor :token
@@ -87,7 +91,7 @@ def app
     # Attributes used for serialization
     # @return [Hash] attributes to be serialized
     def attributes
-      %w(id cluster_id job_id created_at token title script_type cache_completed).map do |attribute|
+      %w(id cluster_id job_id created_at token title script_type cache_completed completed_at).map do |attribute|
         [ attribute, nil ]
       end.to_h
     end
@@ -421,6 +425,7 @@ def update_info
     def update_cache_completed!
       if (! cache_completed) && completed?
         self.cache_completed = true
+        self.completed_at = Time.now.to_i
         db_file.write(to_json)
       end
     end

apps/dashboard/test/models/batch_connect/session_test.rb

@@ -551,7 +551,8 @@ def completed?
           'token'           => 'bc_jupyter',
           'title'           => 'Jupyter Notebook',
           'script_type'     => 'basic',
-          'cache_completed' => nil
+          'cache_completed' => nil,
+          'completed_at'    => nil
         }
         Timecop.freeze(now) do
           assert session.save(app: bc_jupyter_app, context: ctx), session.errors.each(&:to_s).to_s

apps/dashboard/test/system/batch_connect_widgets_test.rb

@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+
+# TODO: Refactor batch_connect_test.rb to include tests that require slurm
+# (testing that things submit) and pull out/write tests for widgets (do not
+# require form submission) into this file - perhaps a batch_connect_test/
+# directory
+
+require 'application_system_test_case'
+require 'ood_core/job/adapters/slurm'
+
+class BatchConnectWidgetsTest < ApplicationSystemTestCase
+  def setup
+    stub_sys_apps
+    stub_user
+    Configuration.stubs(:bc_dynamic_js).returns(true)
+    Configuration.stubs(:bc_dynamic_js?).returns(true) #stub the alias too
+  end
+
+  def stub_git(dir)
+    Open3.stubs(:capture3)
+      .with('git', 'describe', '--always', '--tags', chdir: dir)
+      .returns(['1.2.3', '', exit_success])
+  end
+
+  def make_bc_app(dir, form)
+    SysRouter.stubs(:base_path).returns(Pathname.new(dir))
+    app_dir = "#{dir}/app".tap { |d| Dir.mkdir(d) }
+    stub_scontrol
+    stub_sacctmgr
+    stub_git(app_dir)
+    Pathname.new(app_dir).join('form.yml').write(form)
+  end
+
+  test 'path_selector can be hidden with data-hide-*' do
+    Dir.mktmpdir do |dir|
+      "#{dir}/app".tap { |d| Dir.mkdir(d) }
+      SysRouter.stubs(:base_path).returns(Pathname.new(dir))
+      stub_scontrol
+      stub_sacctmgr
+      stub_git("#{dir}/app")
+
+      form = <<~HEREDOC
+        ---
+        cluster:
+          - owens
+        form:
+          - path
+          - hide_path
+        attributes:
+          path:
+            widget: 'path_selector'
+            directory: "#{Rails.root}"
+          hide_path:
+            widget: 'select'
+            options:
+              - ['show path', 'show path']
+              - ['hide path', 'hide path', data-hide-path: true]
+      HEREDOC
+
+      Pathname.new("#{dir}/app/").join('form.yml').write(form)
+      base_id = 'batch_connect_session_context_path'
+
+      visit new_batch_connect_session_context_url('sys/app')
+
+      select('show path', from: 'batch_connect_session_context_hide_path')
+      assert find('#batch_connect_session_context_path')
+      assert find("[data-target='#batch_connect_session_context_path_path_selector']")
+
+      select('hide path', from: 'batch_connect_session_context_hide_path')
+      refute find('#batch_connect_session_context_path', visible: :hidden).visible?
+      refute find("[data-target='#batch_connect_session_context_path_path_selector']", visible: :hidden).visible?
+    end
+  end
+end

apps/myjobs/Gemfile

@@ -66,4 +66,4 @@ gem 'ood_appkit', '~> 2.0'
 
 # we have to lock rdoc because 6.4 depends on psych 4.0 which breaks with
 # Psych::BadAlias: Cannot load database configuration: Unknown alias: default
-gem 'rdoc', '6.3.3'
+gem 'rdoc', '6.3.4.1'

apps/myjobs/Gemfile.lock

@@ -205,7 +205,7 @@ GEM
     rb-fsevent (0.11.2)
     rb-inotify (0.10.1)
       ffi (~> 1.0)
-    rdoc (6.3.3)
+    rdoc (6.3.4.1)
     redcarpet (3.6.0)
     request_store (1.5.1)
       rack (>= 1.4)
@@ -270,7 +270,7 @@ DEPENDENCIES
   pbs (~> 2.2.1)
   rails (= 6.1.7.6)
   rails-controller-testing
-  rdoc (= 6.3.3)
+  rdoc (= 6.3.4.1)
   sass-rails (~> 5.0)
   sdoc
   sqlite3 (= 1.4.2)

spec/e2e/e2e_helper.rb

@@ -51,7 +51,7 @@ def dist
     major_version = host_inventory['platform_version'].split('.')[0]
     "el#{major_version}"
   when 'amazon'
-    "amzn#{host_inventory['platform_version']}"
+    "amzn#{host_inventory['platform_version'].split('.')[0]}"
   when 'ubuntu'
     "ubuntu-#{host_inventory['platform_version']}"
   when 'debian'