3126: Handle "no local repository" case better.

diff --git a/apps/workbench/test/integration/jobs_test.rb b/apps/workbench/test/integration/jobs_test.rb
index 29bccd9d76b20846cd0d6ce7519b2858a81bcd5a..887e16cf2f6599c29926db62ef54e655da151493 100644 (file)
--- a/apps/workbench/test/integration/jobs_test.rb
+++ b/apps/workbench/test/integration/jobs_test.rb
@@ -95,10 +95,10 @@ class JobsTest < ActionDispatch::IntegrationTest
 
       # Re-running jobs doesn't currently work because the test API
       # server has no git repository to check against.  For now, check
-      # that the correct script version is mentioned in the
-      # Fiddlesticks error message.
+      # that the error message says something appropriate for that
+      # situation.
       if expect_options && use_latest
-        assert_text "Script version #{job['supplied_script_version']} does not resolve to a commit"
+        assert_text "no local repository for active/foo"
       else
         assert_text "Script version #{job['script_version']} does not resolve to a commit"
       end

diff --git a/services/api/app/models/commit.rb b/services/api/app/models/commit.rb
index ee8c55c39200e2bbef3b76e79c2a8d31f8a8cdb7..a6b085722e90fb043a2277f7781727218e8e2559 100644 (file)
--- a/services/api/app/models/commit.rb
+++ b/services/api/app/models/commit.rb
@@ -119,6 +119,9 @@ class Commit < ActiveRecord::Base
       raise ArgumentError.new "invalid sha1 #{sha1}"
     end
     src_gitdir, _ = git_dir_for repo_name
+    unless src_gitdir
+      raise ArgumentError.new "no local repository for #{repo_name}"
+    end
     dst_gitdir = Rails.configuration.git_internal_dir
     must_pipe("echo #{sha1.shellescape}",
               "git --git-dir #{src_gitdir.shellescape} pack-objects -q --revs --stdout",

diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index 41a16bfd5da64bcf6604c4dfae83723068bd40f5..0923a6f4f304863f307e49c69f5648bff3144d89 100644 (file)
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -125,7 +125,7 @@ class Job < ArvadosModel
   end
 
   def ensure_script_version_is_commit
-    if self.state == Running
+    if state == Running
       # Apparently client has already decided to go for it. This is
       # needed to run a local job using a local working directory
       # instead of a commit-ish.
@@ -147,9 +147,12 @@ class Job < ArvadosModel
   end
 
   def tag_version_in_internal_repository
-    if self.state == Running
+    if state == Running
       # No point now. See ensure_script_version_is_commit.
       true
+    elsif errors.any?
+      # Won't be saved, and script_version might not even be valid.
+      true
     elsif new_record? or repository_changed? or script_version_changed?
       uuid_was = uuid
       begin