Skip to content

Commit

Permalink
3081 - auto_environment_variables (#3432)
Browse files Browse the repository at this point in the history
* Launchers form w/ basic auto_environment_variable widget - WIP

* Adds new auto_environment_variable functionality to launchers form

* Comments out unrelated failing test

* Adds ability to update environment variables

* Additional work on auto_environment_variable

* Small method refactor

* Uncomments out test that has been fixed on master

* Changes variable name

* Minor fixes

* Fixes syntax

* Rsolves the problem where if the user chooses their own label,
they can't see the name of the variable in the form.
  • Loading branch information
HazelGrant authored Mar 28, 2024
1 parent fdc57a7 commit 7f32801
Show file tree
Hide file tree
Showing 11 changed files with 133 additions and 4 deletions.
1 change: 1 addition & 0 deletions apps/dashboard/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ gem 'sdoc', group: :doc, require: false
group :development, :test do
# Call 'byebug' anywhere in the code to stop execution and get a debugger console
gem 'byebug'
gem 'pry'
gem 'climate_control', '~> 0.2'
gem 'timecop', '~> 0.9'
end
Expand Down
5 changes: 5 additions & 0 deletions apps/dashboard/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ GEM
xpath (~> 3.2)
childprocess (4.1.0)
climate_control (0.2.0)
coderay (1.1.3)
coffee-rails (5.0.0)
coffee-script (>= 2.2.0)
railties (>= 5.2.0)
Expand Down Expand Up @@ -180,6 +181,9 @@ GEM
ood_support (0.0.5)
pbs (2.2.1)
ffi (~> 1.9, >= 1.9.6)
pry (0.14.2)
coderay (~> 1.1)
method_source (~> 1.0)
psych (5.1.2)
stringio
public_suffix (5.0.4)
Expand Down Expand Up @@ -303,6 +307,7 @@ DEPENDENCIES
ood_core (~> 0.24.1)
ood_support (~> 0.0.2)
pbs (~> 2.2.1)
pry
rails (= 7.0.8.1)
redcarpet (~> 3.3)
rest-client (~> 2.0)
Expand Down
8 changes: 7 additions & 1 deletion apps/dashboard/app/controllers/launchers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,13 @@ def submit_script_params
end

def save_script_params
params.permit({ launcher: SAVE_SCRIPT_KEYS }, :project_id, :id)
auto_env_params = params[:launcher].keys.select do |k|
k.match?('auto_environment_variable')
end

allowlist = SAVE_SCRIPT_KEYS + auto_env_params

params.permit({ launcher: allowlist }, :project_id, :id)
end

def find_project
Expand Down
5 changes: 5 additions & 0 deletions apps/dashboard/app/helpers/launchers_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ def auto_job_name_template
create_editable_widget(script_form_double, attrib)
end

def auto_environment_variable_template
attrib = SmartAttributes::AttributeFactory.build_auto_environment_variable
create_editable_widget(script_form_double, attrib)
end

# We need a form builder to build the template divs. These are
# templates so that they are not a part of the _actual_ form (yet).
# Otherwise you'd have required fields that you cannot actually edit
Expand Down
29 changes: 27 additions & 2 deletions apps/dashboard/app/javascript/launcher_edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
let newFieldTemplate = undefined;

const newFieldData = {
"bc_num_hours": {
bc_num_hours: {
label: "Hours",
help: "How long the job can run for.",
},
auto_queues: {
label: "Queues",
help: "Which queue the job will submit too.",
},
},
auto_accounts: {
label: "Account",
help: "The account the job will be submitted with."
Expand All @@ -22,6 +22,10 @@ const newFieldData = {
bc_num_slots: {
label: "Nodes",
help: "How many nodes the job will run on."
},
auto_environment_variable: {
label: 'Environment Variable',
help: 'Add an environment variable.'
}
}

Expand Down Expand Up @@ -147,11 +151,29 @@ function addInProgressField(event) {
justAdded.find('[data-fixed-toggler]')
.on('click', (event) => { toggleFixedField(event) });

justAdded.find('[data-auto-environment-variable="name"]')
.on('keyup', (event) => { updateAutoEnvironmentVariable(event) });

const entireDiv = event.target.parentElement.parentElement.parentElement;
entireDiv.remove();
enableNewFieldButton();
}

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

input_field.removeAttribute('readonly');
input_field.id = `launcher_auto_environment_variable_${aev_name}`;
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];
label_field.innerHTML = `Environment Variable: ${aev_name}`;
}
}

function fixExcludeBasedOnSelect(selectElement) {
const excludeElementId = selectElement.dataset.excludeId;
const selectOptions = Array.from(selectElement.options);
Expand Down Expand Up @@ -338,6 +360,9 @@ jQuery(() => {
$('[data-fixed-toggler]')
.on('click', (event) => { toggleFixedField(event) });

$('[data-auto-environment-variable="name"]')
.on('keyup', (event) => { updateAutoEnvironmentVariable(event) });

initSelectFields();
initFixedFields();
});
1 change: 1 addition & 0 deletions apps/dashboard/app/lib/smart_attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@ module SmartAttributes
require 'smart_attributes/attributes/bc_queue'
require 'smart_attributes/attributes/bc_vnc_idle'
require 'smart_attributes/attributes/bc_vnc_resolution'
require 'smart_attributes/attributes/auto_environment_variable'
end
5 changes: 5 additions & 0 deletions apps/dashboard/app/lib/smart_attributes/attribute_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module SmartAttributes
class AttributeFactory

AUTO_MODULES_REX = /\Aauto_modules_([\w-]+)\z/.freeze
AUTO_ENVIRONMENT_VARIABLE_REX = /\Aauto_environment_variable_([\w-]+)\z/.freeze

class << self
# Build an attribute object from an id and its options
Expand All @@ -14,6 +15,10 @@ def build(id, opts = {})
hpc_mod = id.match(AUTO_MODULES_REX)[1]
id = 'auto_modules'
opts = opts.merge({'module' => hpc_mod})
elsif id.match?(AUTO_ENVIRONMENT_VARIABLE_REX)
env_variable = id.match(AUTO_ENVIRONMENT_VARIABLE_REX)[1]
id = 'auto_environment_variable'
opts = opts.merge({'key' => env_variable})
end

build_method = "build_#{id}"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
module SmartAttributes
class AttributeFactory
extend AccountCache

def self.build_auto_environment_variable(opts = {})
Attributes::AutoEnvironmentVariable.new('auto_environment_variable', opts)
end
end

module Attributes
class AutoEnvironmentVariable < Attribute
def initialize(id, opts = {})
super

@key = @opts.delete(:key)
# reset the id to be unique from other auto_environment_variable_*
@id = @key ? "#{id}_#{normalize_key(@key)}" : id
end

def widget
'text_field'
end

def original_label
return 'Environment Variable' if !@opts[:label] || opts[:label].match('Environment Variable')
@opts[:label].to_s
end

def label(*)
(opts[:label] || "Environment Variable: #{@key}").to_s
end

def field_options(fmt: nil)
@key.blank? ? super.merge({readonly: true}) : super
end

def normalize_key(key_name)
key_name.to_s.gsub('-', '_')
end

def submit(*)
{ script: { job_environment: { @key.to_s.upcase => value }}}
end
end
end
end
4 changes: 4 additions & 0 deletions apps/dashboard/app/views/launchers/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,7 @@
<template id="auto_job_name_template">
<%= auto_job_name_template %>
</template>

<template id="auto_environment_variable_template">
<%= auto_environment_variable_template %>
</template>
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,15 @@
-%>

<div class="editable-form-field">
<% if field_id.match?('auto_environment_variable') %>
<label>Name</label>
<input class="form-control edit-field"
type="text"
data-auto-environment-variable="name"
data-label-string=<%= attrib.original_label.gsub(' ', '&#32;').force_encoding('utf-8') %>
value=<%= attrib.id.gsub(/auto_environment_variable_?/, '') %>
>
<% end %>
<%= create_widget(form, attrib, format: format, hide_fixed: false) %>

<div class="d-none edit-group mb-3">
Expand Down
24 changes: 23 additions & 1 deletion apps/dashboard/test/system/project_manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@ def add_bc_num_hours(project_id, script_id)
click_on(I18n.t('dashboard.save'))
end

def add_auto_environment_variable(project_id, script_id, save: true)
# now add 'auto_environment_variable'
click_on('Add new option')
select('Environment Variable', from: 'add_new_field_select')
click_on(I18n.t('dashboard.add'))
end

test 'create a new project on fs and display the table entry' do
Dir.mktmpdir do |dir|
project_id = setup_project(dir)
Expand Down Expand Up @@ -442,7 +449,7 @@ def add_bc_num_hours(project_id, script_id)
actual_new_options = page.all("##{new_field_id} option").map(&:value).to_set
expected_new_options = [
'bc_num_hours', 'auto_queues', 'bc_num_slots',
'auto_accounts', 'auto_job_name'
'auto_accounts', 'auto_job_name', 'auto_environment_variable'
].to_set
assert_equal expected_new_options, actual_new_options
end
Expand Down Expand Up @@ -486,6 +493,15 @@ def add_bc_num_hours(project_id, script_id)
find('#launcher_bc_num_hours_fixed').click
find('#save_launcher_bc_num_hours').click

# add auto_environment_variable
add_auto_environment_variable(project_id, script_id)
find('#edit_launcher_auto_environment_variable').click

find("[data-auto-environment-variable='name']").fill_in(with: 'SOME_VARIABLE')
find("#launcher_auto_environment_variable_SOME_VARIABLE").fill_in(with: 'some_value')

find('#save_launcher_auto_environment_variable').click

# correctly saves
click_on(I18n.t('dashboard.save'))
success_message = I18n.t('dashboard.jobs_scripts_updated')
Expand All @@ -501,6 +517,7 @@ def add_bc_num_hours(project_id, script_id)
- auto_scripts
- auto_batch_clusters
- bc_num_hours
- auto_environment_variable_SOME_VARIABLE
attributes:
auto_scripts:
options:
Expand Down Expand Up @@ -530,6 +547,11 @@ def add_bc_num_hours(project_id, script_id)
label: Number of hours
help: ''
required: true
auto_environment_variable_SOME_VARIABLE:
value: some_value
label: 'Environment Variable: SOME_VARIABLE'
help: ''
required: false
HEREDOC

assert_equal(expected_yml, File.read("#{dir}/projects/#{project_id}/.ondemand/scripts/#{script_id}/form.yml"))
Expand Down

0 comments on commit 7f32801

Please sign in to comment.