Skip to content

Commit

Permalink
Launcher save fixed attributes (#3784)
Browse files Browse the repository at this point in the history
Fix auto_scripts so that scripts can be fixed and it populates a default value.
  • Loading branch information
ashton22305 authored Oct 3, 2024
1 parent 7e2ab8b commit d95e1b2
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 14 deletions.
4 changes: 2 additions & 2 deletions apps/dashboard/app/javascript/launcher_edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ function addInProgressField(event) {
}

function updateAutoEnvironmentVariable(event) {
var aev_name = event.target.value;
const aev_name = event.target.value;
const labelString = event.target.dataset.labelString;
var input_field = event.target.parentElement.children[2].children[1];

Expand All @@ -173,7 +173,7 @@ function updateAutoEnvironmentVariable(event) {
input_field.name = `launcher[auto_environment_variable_${aev_name}]`;

if (labelString.match(/Environment( |\s)Variable/)) {
var label_field = event.target.parentElement.children[2].children[0];
const label_field = event.target.parentElement.children[2].children[0];
label_field.innerHTML = `Environment Variable: ${aev_name}`;
}
}
Expand Down
15 changes: 13 additions & 2 deletions apps/dashboard/app/lib/smart_attributes/attributes/auto_scripts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ def self.build_auto_scripts(opts = {})
options = script_options_from_directory(dir)

static_opts = {
options: options
}.merge(opts.without(:options).to_h)
options: options,
value: default_script_value(opts[:value], options)
}.merge(opts.without(:options, :value).to_h)

Attributes::AutoScripts.new('auto_scripts', static_opts)
end
Expand All @@ -26,6 +27,16 @@ def self.script_options_from_directory(dir)
[File.basename(file), file]
end.sort_by(&:first)
end

def self.default_script_value(default, scripts)
if !default || scripts.empty?
nil
elsif scripts.none? { |s| s[0] == File.basename(default) }
scripts.first[1].to_s
else
scripts.select { |s| s[0] == File.basename(default) }.first[1]
end
end
end

module Attributes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@ def self.build_bc_num_slots(opts = {})

module Attributes
class BcNumSlots < Attribute
# Hash of options used to define this attribute
# @return [Hash] attribute options
def opts
@opts.reverse_merge(min: 1, step: 1)
def initialize(id, opts)
super(id, opts)
@opts = @opts.reverse_merge(min: 1, step: 1)
end

# Value of attribute
Expand Down
9 changes: 6 additions & 3 deletions apps/dashboard/app/models/launcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -338,9 +338,12 @@ def add_default_fields(form: [], **_args)
def add_script_to_form(form: [], attributes: {})
form << 'auto_scripts' unless form.include?('auto_scripts')

attributes[:auto_scripts] = {
directory: project_dir
}
dir = { directory: project_dir }
attributes[:auto_scripts] = if attributes[:auto_scripts]
attributes[:auto_scripts].merge(dir)
else
dir
end
end

def add_cluster_to_form(form: [], attributes: {})
Expand Down
10 changes: 9 additions & 1 deletion apps/dashboard/test/lib/smart_attributes/auto_scripts_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def fixture_dir
end

test 'can correctly set the value when scripts directory is configured' do
value = '/test/script/executable.sh'
value = "#{fixture_dir}/script4.slurm"
attribute = SmartAttributes::AttributeFactory.build('auto_scripts', { directory: fixture_dir, value: value })

assert_instance_of SmartAttributes::Attributes::AutoScripts, attribute
Expand Down Expand Up @@ -57,4 +57,12 @@ def fixture_dir
assert_instance_of SmartAttributes::Attributes::AutoScripts, attribute
assert_equal('select', File.basename(attribute.widget))
end

test 'correctly sets value when previous value is invalid' do
value = '/test/invalid/script.sh'
attribute = SmartAttributes::AttributeFactory.build('auto_scripts', { directory: fixture_dir, value: value })

assert_instance_of SmartAttributes::Attributes::AutoScripts, attribute
assert_equal(attribute.select_choices[0].last, attribute.value)
end
end
5 changes: 3 additions & 2 deletions apps/dashboard/test/system/project_manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -582,8 +582,8 @@ def add_auto_environment_variable(project_id, script_id, save: true)
- "#{dir}/projects/#{project_id}/my_cool_script.sh"
- - my_cooler_script.bash
- "#{dir}/projects/#{project_id}/my_cooler_script.bash"
directory: "#{dir}/projects/#{project_id}"
value: "#{dir}/projects/#{project_id}/my_cool_script.sh"
directory: "#{dir}/projects/#{project_id}"
label: Script
help: ''
required: false
Expand Down Expand Up @@ -688,8 +688,8 @@ def add_auto_environment_variable(project_id, script_id, save: true)
- "#{dir}/projects/#{project_id}/my_cool_script.sh"
- - my_cooler_script.bash
- "#{dir}/projects/#{project_id}/my_cooler_script.bash"
directory: "#{dir}/projects/#{project_id}"
value: "#{dir}/projects/#{project_id}/my_cool_script.sh"
directory: "#{dir}/projects/#{project_id}"
label: Script
help: ''
required: false
Expand Down Expand Up @@ -971,6 +971,7 @@ def add_auto_environment_variable(project_id, script_id, save: true)
.stubs(:info).returns(OodCore::Job::Info.new(id: 'job-id-123', status: :running))

find("#launch_8woi7ghd").click

assert_selector('.alert-success', text: 'job-id-123')

# sleep here because this test can error with Errno::ENOTEMPTY: Directory not empty @ dir_s_rmdir
Expand Down

0 comments on commit d95e1b2

Please sign in to comment.