From 286a3f394d6dc545bc1c3760384a566d49fb2dca Mon Sep 17 00:00:00 2001 From: Konrad Hanff Date: Thu, 22 Apr 2021 08:58:50 +0200 Subject: [PATCH] Fix autocorrectable rubocop offences and implement suggestions --- app/errors/application_error.rb | 2 ++ app/errors/runner_not_available_error.rb | 2 ++ app/jobs/application_job.rb | 2 ++ app/jobs/runner_cleanup_job.rb | 6 ++-- app/models/runner.rb | 37 ++++++++++++------------ app/models/submission.rb | 22 +++++++------- config/initializers/jobs.rb | 2 ++ 7 files changed, 42 insertions(+), 31 deletions(-) diff --git a/app/errors/application_error.rb b/app/errors/application_error.rb index abcd35ed..6056921c 100644 --- a/app/errors/application_error.rb +++ b/app/errors/application_error.rb @@ -1,2 +1,4 @@ +# frozen_string_literal: true + class ApplicationError < StandardError end diff --git a/app/errors/runner_not_available_error.rb b/app/errors/runner_not_available_error.rb index 08d651d8..4390fad4 100644 --- a/app/errors/runner_not_available_error.rb +++ b/app/errors/runner_not_available_error.rb @@ -1,2 +1,4 @@ +# frozen_string_literal: true + class RunnerNotAvailableError < ApplicationError end diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb index 9b7f20f2..97fc265f 100644 --- a/app/jobs/application_job.rb +++ b/app/jobs/application_job.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ApplicationJob < ActiveJob::Base queue_as :default end diff --git a/app/jobs/runner_cleanup_job.rb b/app/jobs/runner_cleanup_job.rb index 4a5c172f..f578e9d5 100644 --- a/app/jobs/runner_cleanup_job.rb +++ b/app/jobs/runner_cleanup_job.rb @@ -1,13 +1,15 @@ +# frozen_string_literal: true + class RunnerCleanupJob < ApplicationJob CLEANUP_INTERVAL = CodeOcean::Config.new(:code_ocean).read[:runner_management][:cleanup_interval].seconds - after_perform do |job| + after_perform do |_job| # re-schedule job self.class.set(wait: CLEANUP_INTERVAL).perform_later end def perform - Rails.logger.debug(Time.now) + Rails.logger.debug(Time.zone.now) Runner.inactive_runners.each do |runner| Rails.logger.debug("Destroying runner #{runner.runner_id}, unused since #{runner.last_used}") runner.destroy diff --git a/app/models/runner.rb b/app/models/runner.rb index 22e8bd85..653bc886 100644 --- a/app/models/runner.rb +++ b/app/models/runner.rb @@ -10,20 +10,22 @@ class Runner < ApplicationRecord belongs_to :execution_environment belongs_to :user, polymorphic: true - before_create :get_runner + before_create :new_runner before_destroy :destroy_runner - validates :execution_environment_id, presence: true + validates :execution_environment, presence: true validates :user, presence: true validates :time_limit, presence: true - scope :inactive_runners, -> { where('last_used < ?', Time.now - UNUSED_EXPIRATION_TIME) } - - def self.for(user, exercise, time_limit = 0) + scope :inactive_runners, -> { where('last_used < ?', Time.zone.now - UNUSED_EXPIRATION_TIME) } + + def self.for(user, exercise) execution_environment = ExecutionEnvironment.find(exercise.execution_environment_id) - runner = Runner.find_or_create_by(user: user, execution_environment: execution_environment, time_limit: time_limit) + runner = Runner.find_or_create_by(user: user, execution_environment: execution_environment, + time_limit: execution_environment.permitted_execution_time) return runner if runner.save + raise(RunnerNotAvailableError, 'No runner available') end @@ -31,22 +33,22 @@ class Runner < ApplicationRecord url = "#{runner_url}/files" body = {files: files.map { |filename, content| {filepath: filename, content: content} }} response = Faraday.patch(url, body.to_json, HEADERS) - if response.status == 404 - # runner has disappeared for some reason - self.destroy - raise(RunnerNotAvailableError, "Runner unavailable") - end + return unless response.status == 404 + + # runner has disappeared for some reason + destroy + raise(RunnerNotAvailableError, 'Runner unavailable') end def execute_command(command) + used_now url = "#{runner_url}/execute" response = Faraday.post(url, {command: command}.to_json, HEADERS) if response.status == 404 # runner has disappeared for some reason - self.destroy - raise(RunnerNotAvailableError, "Runner unavailable") + destroy + raise(RunnerNotAvailableError, 'Runner unavailable') end - used_now parse response end @@ -71,10 +73,9 @@ class Runner < ApplicationRecord private - def get_runner + def new_runner url = "#{BASE_URL}/runners" - body = {executionEnvironmentId: execution_environment.id} - body[:timeLimit] = time_limit + body = {executionEnvironmentId: execution_environment.id, timeLimit: time_limit} response = Faraday.post(url, body.to_json, HEADERS) response_body = parse response self.runner_id = response_body[:runnerId] @@ -90,7 +91,7 @@ class Runner < ApplicationRecord end def used_now - self.last_used = Time.now + self.last_used = Time.zone.now save end end diff --git a/app/models/submission.rb b/app/models/submission.rb index 7d14e87d..1a529630 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -72,7 +72,7 @@ class Submission < ApplicationRecord # expects the full file path incl. file extension # Caution: There must be no unnecessary path prefix included. # Use `file.ext` rather than `./file.ext` - collect_files.detect {|file| file.filepath == file_path } + collect_files.detect { |file| file.filepath == file_path } end def normalized_score @@ -144,18 +144,18 @@ class Submission < ApplicationRecord prepared_runner do |runner| scores = collect_files.select(&:teacher_defined_assessment?).map do |file| score_command = command_for execution_environment.test_command, file.name_with_extension - stdout = "" - stderr = "" + stdout = '' + stderr = '' exit_code = 1 # default to error - execution_time = runner.execute_interactively(score_command) do |runner, socket| + execution_time = runner.execute_interactively(score_command) do |_runner, socket| socket.on :stderr do |data| stderr << data end socket.on :stdout do |data| stdout << data end - socket.on :exit do |_exit_code| - exit_code = _exit_code + socket.on :exit do |received_exit_code| + exit_code = received_exit_code EventMachine.stop_event_loop end end @@ -163,9 +163,9 @@ class Submission < ApplicationRecord file_role: file.role, waiting_for_container_time: runner.waiting_time, container_execution_time: execution_time, - status: (exit_code == 0) ? :ok : :failed, + status: exit_code.zero? ? :ok : :failed, stdout: stdout, - stderr: stderr, + stderr: stderr } test_result(output, file) end @@ -194,10 +194,10 @@ class Submission < ApplicationRecord end def prepared_runner - request_time = Time.now - runner = Runner.for(user, exercise, execution_environment.permitted_execution_time) + request_time = Time.zone.now + runner = Runner.for(user, exercise) copy_files_to runner - runner.waiting_time = Time.now - request_time + runner.waiting_time = Time.zone.now - request_time yield(runner) if block_given? end diff --git a/config/initializers/jobs.rb b/config/initializers/jobs.rb index 68eb2e92..f4bf7be2 100644 --- a/config/initializers/jobs.rb +++ b/config/initializers/jobs.rb @@ -1 +1,3 @@ +# frozen_string_literal: true + RunnerCleanupJob.perform_now unless Rake.application.top_level_tasks.to_s.match?(/db:|assets:/)