From 7f328014f02a9c37221092965802aebf0d5b52ed Mon Sep 17 00:00:00 2001 From: Hazel Date: Thu, 28 Mar 2024 11:24:00 -0400 Subject: [PATCH] 3081 - auto_environment_variables (#3432) * 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. --- apps/dashboard/Gemfile | 1 + apps/dashboard/Gemfile.lock | 5 ++ .../app/controllers/launchers_controller.rb | 8 +++- .../dashboard/app/helpers/launchers_helper.rb | 5 ++ .../dashboard/app/javascript/launcher_edit.js | 29 +++++++++++- apps/dashboard/app/lib/smart_attributes.rb | 1 + .../lib/smart_attributes/attribute_factory.rb | 5 ++ .../attributes/auto_environment_variable.rb | 46 +++++++++++++++++++ .../app/views/launchers/edit.html.erb | 4 ++ .../_editable_text_field.erb | 9 ++++ .../test/system/project_manager_test.rb | 24 +++++++++- 11 files changed, 133 insertions(+), 4 deletions(-) create mode 100644 apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb diff --git a/apps/dashboard/Gemfile b/apps/dashboard/Gemfile index 65ad4a5b6a..0de0ff1f6e 100644 --- a/apps/dashboard/Gemfile +++ b/apps/dashboard/Gemfile @@ -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 diff --git a/apps/dashboard/Gemfile.lock b/apps/dashboard/Gemfile.lock index d941a5b720..666125c3c3 100644 --- a/apps/dashboard/Gemfile.lock +++ b/apps/dashboard/Gemfile.lock @@ -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) @@ -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) @@ -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) diff --git a/apps/dashboard/app/controllers/launchers_controller.rb b/apps/dashboard/app/controllers/launchers_controller.rb index f968e59083..519b7c7829 100644 --- a/apps/dashboard/app/controllers/launchers_controller.rb +++ b/apps/dashboard/app/controllers/launchers_controller.rb @@ -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 diff --git a/apps/dashboard/app/helpers/launchers_helper.rb b/apps/dashboard/app/helpers/launchers_helper.rb index 1dd1b313ab..0318099258 100644 --- a/apps/dashboard/app/helpers/launchers_helper.rb +++ b/apps/dashboard/app/helpers/launchers_helper.rb @@ -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 diff --git a/apps/dashboard/app/javascript/launcher_edit.js b/apps/dashboard/app/javascript/launcher_edit.js index 96c68485bc..fdd1c065dd 100644 --- a/apps/dashboard/app/javascript/launcher_edit.js +++ b/apps/dashboard/app/javascript/launcher_edit.js @@ -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." @@ -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.' } } @@ -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); @@ -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(); }); diff --git a/apps/dashboard/app/lib/smart_attributes.rb b/apps/dashboard/app/lib/smart_attributes.rb index 0fd17482d7..86cdc5b435 100644 --- a/apps/dashboard/app/lib/smart_attributes.rb +++ b/apps/dashboard/app/lib/smart_attributes.rb @@ -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 diff --git a/apps/dashboard/app/lib/smart_attributes/attribute_factory.rb b/apps/dashboard/app/lib/smart_attributes/attribute_factory.rb index 433f9d496d..d598d460bc 100644 --- a/apps/dashboard/app/lib/smart_attributes/attribute_factory.rb +++ b/apps/dashboard/app/lib/smart_attributes/attribute_factory.rb @@ -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 @@ -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}" diff --git a/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb new file mode 100644 index 0000000000..8d59af0907 --- /dev/null +++ b/apps/dashboard/app/lib/smart_attributes/attributes/auto_environment_variable.rb @@ -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 \ No newline at end of file diff --git a/apps/dashboard/app/views/launchers/edit.html.erb b/apps/dashboard/app/views/launchers/edit.html.erb index 383448e8d7..d1196fceb2 100644 --- a/apps/dashboard/app/views/launchers/edit.html.erb +++ b/apps/dashboard/app/views/launchers/edit.html.erb @@ -34,3 +34,7 @@ + + \ No newline at end of file diff --git a/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb b/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb index ffc6ae708e..b63aacfe12 100644 --- a/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb +++ b/apps/dashboard/app/views/launchers/editable_form_fields/_editable_text_field.erb @@ -3,6 +3,15 @@ -%>
+<% if field_id.match?('auto_environment_variable') %> + + + value=<%= attrib.id.gsub(/auto_environment_variable_?/, '') %> + > +<% end %> <%= create_widget(form, attrib, format: format, hide_fixed: false) %>
diff --git a/apps/dashboard/test/system/project_manager_test.rb b/apps/dashboard/test/system/project_manager_test.rb index ff48bc7d57..f5fdaf0ce3 100644 --- a/apps/dashboard/test/system/project_manager_test.rb +++ b/apps/dashboard/test/system/project_manager_test.rb @@ -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) @@ -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 @@ -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') @@ -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: @@ -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"))