From d2dfb73a94413b2f2073ae5f96781faaa4b88929 Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Tue, 20 Aug 2024 15:07:14 -0400 Subject: [PATCH 1/4] refactor how jobs are logged in the project manager --- .../app/controllers/projects_controller.rb | 2 +- .../app/models/concerns/job_logger.rb | 48 ++++++++++++++++++ apps/dashboard/app/models/launcher.rb | 50 +------------------ apps/dashboard/app/models/project.rb | 25 ++++++---- 4 files changed, 66 insertions(+), 59 deletions(-) create mode 100644 apps/dashboard/app/models/concerns/job_logger.rb diff --git a/apps/dashboard/app/controllers/projects_controller.rb b/apps/dashboard/app/controllers/projects_controller.rb index a7c49edfdf..13ffd1f2a7 100644 --- a/apps/dashboard/app/controllers/projects_controller.rb +++ b/apps/dashboard/app/controllers/projects_controller.rb @@ -108,7 +108,7 @@ def job_details cluster = OodAppkit.clusters[cluster_str.to_sym] render(:status => 404) if cluster.nil? - hpc_job = project.job_from_id(job_details_params[:jobid].to_s, cluster_str) + hpc_job = project.job(job_details_params[:jobid].to_s, cluster_str) render(partial: 'job_details', locals: { job: hpc_job }) end diff --git a/apps/dashboard/app/models/concerns/job_logger.rb b/apps/dashboard/app/models/concerns/job_logger.rb new file mode 100644 index 0000000000..fc4dd602a5 --- /dev/null +++ b/apps/dashboard/app/models/concerns/job_logger.rb @@ -0,0 +1,48 @@ + +module JobLogger + + def upsert_job!(directory, job) + existing_jobs = jobs(directory) + stored_job = existing_jobs.detect { |j| j.id == job.id && j.cluster == job.cluster } + + if stored_job.nil? + new_jobs = (existing_jobs + [job.to_h]).map(&:to_h) + else + new_jobs = existing_jobs.map(&:to_h) + idx = existing_jobs.index(stored_job) + new_jobs[idx].merge!(job.to_h) { |_key, old_val, new_val| new_val.nil? ? old_val : new_val } + end + + JobLoggerHelper.write_log(directory, new_jobs) + end + + # def write_job_log!(directory, jobs) + # JobLoggerHelper.write_log!(directory, jobs) + # endd + + def jobs(directory) + file = JobLoggerHelper.log_file(directory) + begin + data = YAML.safe_load(File.read(file.to_s), permitted_classes: [Time]).to_a + data.map { |job_data| HpcJob.new(**job_data) } + rescue StandardError => e + Rails.logger.error("Cannot read job log file '#{file}' due to error: #{e}") + [] + end + end + + # helper methods here are located here so that they don't + # bleed into the class that uses this module + class JobLoggerHelper + class << self + def log_file(directory) + Pathname.new("#{directory}/.ondemand/job_log.yml") + end + + def write_log(directory, jobs) + file = log_file(directory) + File.write(file.to_s, jobs.to_yaml) + end + end + end +end diff --git a/apps/dashboard/app/models/launcher.rb b/apps/dashboard/app/models/launcher.rb index 9864896008..bed5f2838a 100644 --- a/apps/dashboard/app/models/launcher.rb +++ b/apps/dashboard/app/models/launcher.rb @@ -2,6 +2,7 @@ class Launcher include ActiveModel::Model + include JobLogger class ClusterNotFound < StandardError; end @@ -196,28 +197,6 @@ def submit(options) nil end - def active_jobs - @active_jobs ||= jobs.reject { |job| job.completed? } - end - - def jobs - @jobs ||= begin - data = YAML.safe_load(File.read(job_log_file.to_s), permitted_classes: [Time]).to_a - data.map do |job_data| - HpcJob.new(**job_data) - end - end - end - - # When a job is requested, update before returning - def job_from_id(job_id, cluster) - job = stored_job_from_id(job_id, cluster) - unless job.nil? || job.status.to_s == 'completed' - update_job_log(job_id, cluster) - end - stored_job_from_id(job_id, cluster) - end - def create_default_script return false if Launcher.scripts?(project_dir) || default_script_path.exist? @@ -310,37 +289,12 @@ def cached_values end end - def most_recent_job - jobs.sort_by do |data| - data['submit_time'] - end.reverse.first.to_h - end - - def stored_job_from_id(job_id, cluster) - jobs.detect { |j| j.id == job_id && j.cluster == cluster } - end - def update_job_log(job_id, cluster) adapter = adapter(cluster).job_adapter info = adapter.info(job_id) job = HpcJob.from_core_info(info: info, cluster: cluster) - existing_jobs = jobs - stored_job = stored_job_from_id(job_id, cluster) - if stored_job.nil? - new_jobs = (jobs + [job.to_h]).map(&:to_h) - else - new_jobs = existing_jobs.map(&:to_h) - idx = existing_jobs.index(stored_job) - new_jobs[idx].merge!(job.to_h) { |key, old_val, new_val| new_val.nil? ? old_val : new_val } - end - - File.write(job_log_file.to_s, new_jobs.to_yaml) - end - def job_log_file - @job_log_file ||= Pathname.new(File.join(Launcher.script_path(project_dir, id), "job_history.log")).tap do |path| - FileUtils.touch(path.to_s) - end + upsert_job!(project_dir, job) end def submit_opts(options, render_format) diff --git a/apps/dashboard/app/models/project.rb b/apps/dashboard/app/models/project.rb index 33ab03a8e2..041301a30b 100644 --- a/apps/dashboard/app/models/project.rb +++ b/apps/dashboard/app/models/project.rb @@ -6,6 +6,7 @@ class Project include ActiveModel::Validations include ActiveModel::Validations::Callbacks include IconWithUri + extend JobLogger class << self def lookup_file @@ -197,18 +198,22 @@ def size end def jobs - launchers = Launcher.all(directory) - launchers.map do |launcher| - launcher.jobs - end.flatten + Project.jobs(directory) end - def job_from_id(job_id, cluster) - launchers = Launcher.all(directory) - launchers.each do |launcher| - job = launcher.job_from_id(job_id, cluster) - return job unless job.nil? - end + def job(job_id, cluster) + cached_job = jobs.detect { |j| j.id == job_id && j.cluster == cluster } + return cached_job if cached_job.completed? + + active_job = adapter(cluster).info(job_id) + active_job = HpcJob.new(**active_job.to_h) + Project.upsert_job!(directory, active_job) + active_job + end + + def adapter(cluster_id) + cluster = OodAppkit.clusters[cluster_id] || raise(StandardError, "Job specifies nonexistent '#{cluster_id}' cluster id.") + cluster.job_adapter end def readme_path From 0f4f902f6062c24093d569efdc3d06be83a8b817 Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Thu, 22 Aug 2024 13:11:28 -0400 Subject: [PATCH 2/4] fix tests --- .../test/system/project_manager_test.rb | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/apps/dashboard/test/system/project_manager_test.rb b/apps/dashboard/test/system/project_manager_test.rb index f6fe41821c..93eb9028d2 100644 --- a/apps/dashboard/test/system/project_manager_test.rb +++ b/apps/dashboard/test/system/project_manager_test.rb @@ -337,15 +337,16 @@ def add_auto_environment_variable(project_id, script_id, save: true) project_id = setup_project(dir) script_id = setup_script(project_id) project_dir = File.join(dir, 'projects', project_id) - script_dir = File.join(project_dir, '.ondemand', 'scripts', script_id) + ondemand_dir = File.join(project_dir, '.ondemand') + script_dir = File.join(ondemand_dir, 'scripts', script_id) # ASSERT SCRIPT DIRECTORY IS CREATED assert_equal true, File.directory?(script_dir) - expected_script_files = ["#{script_dir}/form.yml", "#{script_dir}/job_history.log"] + expected_script_files = ["#{script_dir}/form.yml", "#{ondemand_dir}/job_log.yml"] # ASSERT EXPECTED SCRIPT FILES expected_script_files.each do |file_path| - assert_equal true, File.exist?(file_path) + assert_equal true, File.exist?(file_path), "#{file_path} does not exist" end accept_confirm do @@ -363,7 +364,7 @@ def add_auto_environment_variable(project_id, script_id, save: true) project_id = setup_project(dir) script_id = setup_script(project_id) project_dir = File.join(dir, 'projects', project_id) - script_dir = File.join(project_dir, '.ondemand', 'scripts', script_id) + ondemand_dir = File.join(project_dir, '.ondemand') add_account(project_id, script_id) launcher_path = project_launcher_path(project_id, script_id) @@ -374,7 +375,7 @@ def add_auto_environment_variable(project_id, script_id, save: true) assert_equal 'oakley', find('#launcher_auto_batch_clusters').value assert_equal 'pzs0715', find('#launcher_auto_accounts').value assert_equal "#{project_dir}/my_cool_script.sh", find('#launcher_auto_scripts').value - assert_nil YAML.safe_load(File.read("#{script_dir}/job_history.log")) + assert_nil YAML.safe_load(File.read("#{ondemand_dir}/job_log.yml")) select('owens', from: 'launcher_auto_batch_clusters') select('pas2051', from: 'launcher_auto_accounts') @@ -391,7 +392,7 @@ def add_auto_environment_variable(project_id, script_id, save: true) click_on 'Launch' assert_selector('.alert-success', text: 'job-id-123') - jobs = YAML.safe_load(File.read("#{script_dir}/job_history.log"), permitted_classes: [Time]) + jobs = YAML.safe_load(File.read("#{ondemand_dir}/job_log.yml"), permitted_classes: [Time]) assert_equal(1, jobs.size) assert_equal('job-id-123', jobs[0]['id']) @@ -404,7 +405,7 @@ def add_auto_environment_variable(project_id, script_id, save: true) project_id = setup_project(dir) script_id = setup_script(project_id) project_dir = File.join(dir, 'projects', project_id) - script_dir = File.join(project_dir, '.ondemand', 'scripts', script_id) + ondemand_dir = File.join(project_dir, '.ondemand') add_account(project_id, script_id, save: false) click_on('Add new option') @@ -421,7 +422,7 @@ def add_auto_environment_variable(project_id, script_id, save: true) assert_equal 'oakley', find('#launcher_auto_batch_clusters').value assert_equal 'pzs0715', find('#launcher_auto_accounts').value assert_equal "#{project_dir}/my_cool_script.sh", find('#launcher_auto_scripts').value - assert_nil YAML.safe_load(File.read("#{script_dir}/job_history.log")) + assert_nil YAML.safe_load(File.read("#{ondemand_dir}/job_log.yml")) select('owens', from: 'launcher_auto_batch_clusters') select('pas2051', from: 'launcher_auto_accounts') @@ -439,7 +440,7 @@ def add_auto_environment_variable(project_id, script_id, save: true) click_on 'Launch' assert_selector('.alert-success', text: 'job-id-123') - jobs = YAML.safe_load(File.read("#{script_dir}/job_history.log"), permitted_classes: [Time]) + jobs = YAML.safe_load(File.read("#{ondemand_dir}/job_log.yml"), permitted_classes: [Time]) assert_equal(1, jobs.size) assert_equal('job-id-123', jobs[0]['id']) @@ -451,7 +452,7 @@ def add_auto_environment_variable(project_id, script_id, save: true) project_id = setup_project(dir) script_id = setup_script(project_id) project_dir = File.join(dir, 'projects', project_id) - script_dir = File.join(project_dir, '.ondemand', 'scripts', script_id) + ondemand_dir = File.join(project_dir, '.ondemand') add_account(project_id, script_id) launcher_path = project_launcher_path(project_id, script_id) @@ -462,7 +463,7 @@ def add_auto_environment_variable(project_id, script_id, save: true) assert_equal 'oakley', find('#launcher_auto_batch_clusters').value assert_equal 'pzs0715', find('#launcher_auto_accounts').value assert_equal "#{project_dir}/my_cool_script.sh", find('#launcher_auto_scripts').value - assert_nil YAML.safe_load(File.read("#{script_dir}/job_history.log")) + assert_nil YAML.safe_load(File.read("#{ondemand_dir}/job_log.yml")) select('owens', from: 'launcher_auto_batch_clusters') select('pas2051', from: 'launcher_auto_accounts') @@ -476,7 +477,7 @@ def add_auto_environment_variable(project_id, script_id, save: true) click_on 'Launch' assert_selector('.alert-danger', text: "Close\nsome error message") - assert_nil YAML.safe_load(File.read("#{script_dir}/job_history.log")) + assert_nil YAML.safe_load(File.read("#{ondemand_dir}/job_log.yml")) end end From 674e488122f09b0d7fcd69220e382128caa19273 Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Thu, 22 Aug 2024 13:45:23 -0400 Subject: [PATCH 3/4] need to init the file --- apps/dashboard/app/models/concerns/job_logger.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/dashboard/app/models/concerns/job_logger.rb b/apps/dashboard/app/models/concerns/job_logger.rb index fc4dd602a5..e62d0d0c10 100644 --- a/apps/dashboard/app/models/concerns/job_logger.rb +++ b/apps/dashboard/app/models/concerns/job_logger.rb @@ -36,7 +36,9 @@ def jobs(directory) class JobLoggerHelper class << self def log_file(directory) - Pathname.new("#{directory}/.ondemand/job_log.yml") + Pathname.new("#{directory}/.ondemand/job_log.yml").tap do |path| + FileUtils.touch(path.to_s) unless path.exist? + end end def write_log(directory, jobs) From f05bef4e2e74e664e18bff836941feb880da792c Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Fri, 23 Aug 2024 15:21:47 -0400 Subject: [PATCH 4/4] cast to HpcJob so it has cluster --- apps/dashboard/app/models/project.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/dashboard/app/models/project.rb b/apps/dashboard/app/models/project.rb index 041301a30b..4179cbc517 100644 --- a/apps/dashboard/app/models/project.rb +++ b/apps/dashboard/app/models/project.rb @@ -205,10 +205,10 @@ def job(job_id, cluster) cached_job = jobs.detect { |j| j.id == job_id && j.cluster == cluster } return cached_job if cached_job.completed? - active_job = adapter(cluster).info(job_id) - active_job = HpcJob.new(**active_job.to_h) - Project.upsert_job!(directory, active_job) - active_job + info = adapter(cluster).info(job_id) + job = HpcJob.from_core_info(info: info, cluster: cluster) + Project.upsert_job!(directory, job) + job end def adapter(cluster_id)