[ARVADOS] updated: ebc3fddd97628a50148ed90531b0a18d1cc867f9

git at public.curoverse.com git at public.curoverse.com
Thu Jun 19 13:25:32 EDT 2014


Summary of changes:
 doc/api/methods/jobs.html.textile.liquid           |  31 ++-
 .../api/app/controllers/application_controller.rb  |  11 +-
 .../app/controllers/arvados/v1/jobs_controller.rb  |  99 +++++++-
 services/api/app/models/collection.rb              |  34 +--
 services/api/test/fixtures/jobs.yml                |  16 ++
 services/api/test/fixtures/links.yml               |  10 +-
 .../arvados/v1/job_reuse_controller_test.rb        | 273 +++++++++++++++++++++
 7 files changed, 430 insertions(+), 44 deletions(-)

       via  ebc3fddd97628a50148ed90531b0a18d1cc867f9 (commit)
       via  d9d357302419430ca762a9eb56ef5fda41fe47b9 (commit)
       via  b832c418442822700c684a31e04d5c095735ce3b (commit)
       via  032b8b8c489650409dab16ec49ca853016048b24 (commit)
       via  6d6b609a1952bd0edb87413f24753d7b85c093b7 (commit)
       via  784e112f3fcad487c9f359413fb0dcd8b09c3edb (commit)
       via  613773f364adb36de6b061ba5a78a6df0a53df4f (commit)
       via  8c9b4a1c7b4ad3feb5c0ccb14888aad7ed262e18 (commit)
      from  2fd0eba4e138bd9cadbdf03ea2ca37bbc3f87f24 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit ebc3fddd97628a50148ed90531b0a18d1cc867f9
Merge: 2fd0eba d9d3573
Author: Brett Smith <brett at curoverse.com>
Date:   Thu Jun 19 13:26:03 2014 -0400

    Merge branch '2879-docker-image-job-reuse'
    
    Closes #2879, #2924, #3030.


commit d9d357302419430ca762a9eb56ef5fda41fe47b9
Author: Brett Smith <brett at curoverse.com>
Date:   Thu Jun 19 10:49:56 2014 -0400

    2879: Docker filters on Jobs accept an array of search terms.

diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index 7ce5a62..d38f257 100644
--- a/services/api/app/controllers/arvados/v1/jobs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/jobs_controller.rb
@@ -190,9 +190,11 @@ class Arvados::V1::JobsController < ApplicationController
         false
       when ["docker_image_locator", "in docker"], ["docker_image_locator", "not in docker"]
         filter[1].sub!(/ docker$/, '')
-        image_search, image_tag = filter[2].split(':', 2)
-        filter[2] = Collection.
-          uuids_for_docker_image(image_search, image_tag, @read_users)
+        search_list = filter[2].is_a?(Enumerable) ? filter[2] : [filter[2]]
+        filter[2] = search_list.flat_map do |search_term|
+          image_search, image_tag = search_term.split(':', 2)
+          Collection.uuids_for_docker_image(image_search, image_tag, @read_users)
+        end
         true
       else
         true
diff --git a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
index 6a3fbe4..b00fbf1 100644
--- a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
@@ -529,4 +529,26 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
     refute_includes(assigns(:objects).map { |job| job.uuid },
                     jobs(:previous_job_run).uuid)
   end
+
+  test "'in docker' filter accepts arrays" do
+    get :index, filters: [["docker_image_locator", "in docker",
+                           ["_nonesuchname_", "arvados/apitestfixture"]]]
+    assert_response :success
+    assert_not_nil assigns(:objects)
+    assert_includes(assigns(:objects).map { |job| job.uuid },
+                    jobs(:previous_docker_job_run).uuid)
+    refute_includes(assigns(:objects).map { |job| job.uuid },
+                    jobs(:previous_job_run).uuid)
+  end
+
+  test "'not in docker' filter accepts arrays" do
+    get :index, filters: [["docker_image_locator", "not in docker",
+                           ["_nonesuchname_", "arvados/apitestfixture"]]]
+    assert_response :success
+    assert_not_nil assigns(:objects)
+    assert_includes(assigns(:objects).map { |job| job.uuid },
+                    jobs(:previous_job_run).uuid)
+    refute_includes(assigns(:objects).map { |job| job.uuid },
+                    jobs(:previous_docker_job_run).uuid)
+  end
 end

commit b832c418442822700c684a31e04d5c095735ce3b
Author: Brett Smith <brett at curoverse.com>
Date:   Thu Jun 19 13:24:47 2014 -0400

    2879: Job Docker filters respect reader tokens.

diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index 3e3150b..7ce5a62 100644
--- a/services/api/app/controllers/arvados/v1/jobs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/jobs_controller.rb
@@ -190,7 +190,9 @@ class Arvados::V1::JobsController < ApplicationController
         false
       when ["docker_image_locator", "in docker"], ["docker_image_locator", "not in docker"]
         filter[1].sub!(/ docker$/, '')
-        filter[2] = Collection.uuids_for_docker_image(*filter[2].split(':', 2))
+        image_search, image_tag = filter[2].split(':', 2)
+        filter[2] = Collection.
+          uuids_for_docker_image(image_search, image_tag, @read_users)
         true
       else
         true
diff --git a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
index 5cdf6c7..6a3fbe4 100644
--- a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
@@ -514,4 +514,19 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
     refute_includes(assigns(:objects).map { |job| job.uuid },
                     jobs(:previous_job_run).uuid)
   end
+
+  test "find Job with Docker image using reader tokens" do
+    authorize_with :inactive
+    get(:index, {
+          filters: [["docker_image_locator", "in docker",
+                     "arvados/apitestfixture"]],
+          reader_tokens: [api_token(:active)],
+        })
+    assert_response :success
+    assert_not_nil assigns(:objects)
+    assert_includes(assigns(:objects).map { |job| job.uuid },
+                    jobs(:previous_docker_job_run).uuid)
+    refute_includes(assigns(:objects).map { |job| job.uuid },
+                    jobs(:previous_job_run).uuid)
+  end
 end

commit 032b8b8c489650409dab16ec49ca853016048b24
Author: Brett Smith <brett at curoverse.com>
Date:   Tue Jun 17 13:52:52 2014 -0400

    2879: Document new Job API filters.

diff --git a/doc/api/methods/jobs.html.textile.liquid b/doc/api/methods/jobs.html.textile.liquid
index d75977a..879feb4 100644
--- a/doc/api/methods/jobs.html.textile.liquid
+++ b/doc/api/methods/jobs.html.textile.liquid
@@ -34,28 +34,37 @@ table(table table-bordered table-condensed).
 |minimum_script_version |string     |Git branch, tag, or commit hash specifying the minimum acceptable script version (earliest ancestor) to consider when deciding whether to re-use a past job.[1]|query|@"c3e86c9"@|
 |exclude_script_versions|array of strings|Git commit branches, tags, or hashes to exclude when deciding whether to re-use a past job.|query|@["8f03c71","8f03c71"]@
 @["badtag1","badtag2"]@|
+|filters|array|Conditions to find Jobs to reuse.|query||
 |find_or_create         |boolean    |Before creating, look for an existing job that has identical script, script_version, and script_parameters to those in the present job, has nondeterministic=false, and did not fail (it could be queued, running, or completed). If such a job exists, respond with the existing job instead of submitting a new one.|query|@false@|
 
 When a job is submitted to the queue using the **create** method, the @script_version@ attribute is updated to a full 40-character Git commit hash based on the current content of the specified repository. If @script_version@ cannot be resolved, the job submission is rejected.
 
 fn1. See the "note about specifying Git commits on the Job resource page":{{site.baseurl}}/api/schema/Job.html#script_version for more detail.
 
+h3. Specialized filters
+
+Special filter operations are available for specific Job columns.
+
+* @script_version@ @in git@ @REFSPEC@<br>Resolve @REFSPEC@ to a list of git commits, and match jobs with a @script_version@ in that list.  The create method will find commits between @REFSPEC@ and the submitted job's @script_version@; the list method will search between @REFSPEC@ and HEAD.  This list may include parallel branches if there is more than one path between @REFSPEC@ and the end commit in the graph.  Use @not in@ or @not in git@ filters (below) to blacklist specific commits.
+
+* @script_version@ @not in git@ @REFSPEC@<br>Resolve @REFSPEC@ to a list of git commits, and match jobs with a @script_version@ not in that list.
+
+* @docker_image_locator@ @in docker@ @SEARCH@<br>@SEARCH@ can be a Docker image hash, a repository name, or a repository name and tag separated by a colon (@:@).  The server will find collections that contain a Docker image that match that search criteria, then match jobs with a @docker_image_locator@ in that list.
+
+* @docker_image_locator@ @not in docker@ @SEARCH@<br>Negate the @in docker@ filter.
+
 h3. Reusing jobs
 
-Because Arvados records the exact version of the script, input parameters, and runtime environment [1] that was used to run the job, if the script is deterministic (meaning that the same code version is guaranteed to produce the same outputs from the same inputs) then it is possible to re-use the results of past jobs, and avoid re-running the computation to save time.  Arvados uses the following algorithm to determine if a past job can be re-used:
+Because Arvados records the exact version of the script, input parameters, and runtime environment that was used to run the job, if the script is deterministic (meaning that the same code version is guaranteed to produce the same outputs from the same inputs) then it is possible to re-use the results of past jobs, and avoid re-running the computation to save time.  Arvados uses the following algorithm to determine if a past job can be re-used:
 
 notextile. <div class="spaced-out">
 
 # If @find_or_create@ is false or omitted, create a new job and skip the rest of these steps.
-# Find a list of acceptable values for @script_version at .  If @minimum_script_version@ is specified, this is the set of all revisions in the Git commit graph between @minimum_script_version@ and the @script_version@ in the submitted "job object":{{site.baseurl}}/api/schema/Job.html (inclusive)[2].  If @minimum_script_version@ is not specified, only @script_version@ is added to the list.  If @exclude_script_versions@ is specified, the listed versions are excluded from the list.
-# Select jobs whose @script@ and @script_parameters@ attributes match those in the submitted "job object":{{site.baseurl}}/api/schema/Job.html, and whose @script_version@ attribute is in the list of acceptable versions.  Exclude jobs that failed or set @nondeterministic@ to true.
-# If more than one of the candidate jobs has finished, check that all such jobs actually did produce the same output.
-# If existing jobs exist and do not disagree with one another about the correct output, return one of the selected past jobs instead of creating a new job. If there is more than one match, which job will be returned is undefined.
-# If an existing job could not be chosen this way, create a new job.
-
-fn1. As of this writing, versioning the runtime environment is still under development.
-
-fn2. This may include parallel branches if there is more than one path between @minimum_script_version@ and the submitted job's @script_version@ in the Git commit graph.  Use @exclude_script_versions@ to blacklist specific commits.
+# If @filters@ are specified, find jobs that match those filters.  Filters *must* be specified to limit the @repository@ and @script@ attributes.  An error is returned if they are missing.
+# If @filters@ are not specified, find jobs with the same @repository@ and @script@, with a @script_version@ between @minimum_script_version@ and @script_version@ (excluding @excluded_script_versions@), and a @docker_image_locator@ with the latest Collection that matches the submitted job's @docker_image@ constraint.
+# If the found jobs include a completed job, and all found completed jobs have consistent output, return one of them.  Which specific job is returned is undefined.
+# If the found jobs only include incomplete jobs, return one of them.  Which specific job is returned is undefined.
+# If no job has been returned so far, create and return a new job.
 
 </div>
 
@@ -159,6 +168,8 @@ table(table table-bordered table-condensed).
 |order|string|Order in which to return matching jobs.|query||
 |filters|array|Conditions for filtering jobs.|query||
 
+See the create method documentation for more information about Job-specific filters.
+
 h2. log_tail_follow
 
 log_tail_follow jobs

commit 6d6b609a1952bd0edb87413f24753d7b85c093b7
Author: Brett Smith <brett at curoverse.com>
Date:   Tue Jun 17 13:51:18 2014 -0400

    2879: Support tag search in "in docker" Job filters.
    
    The search uses the same `reponame:tagname` format used by
    `docker tag`.

diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index d823b00..3e3150b 100644
--- a/services/api/app/controllers/arvados/v1/jobs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/jobs_controller.rb
@@ -190,7 +190,7 @@ class Arvados::V1::JobsController < ApplicationController
         false
       when ["docker_image_locator", "in docker"], ["docker_image_locator", "not in docker"]
         filter[1].sub!(/ docker$/, '')
-        filter[2] = Collection.uuids_for_docker_image(filter[2])
+        filter[2] = Collection.uuids_for_docker_image(*filter[2].split(':', 2))
         true
       else
         true
diff --git a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
index 450093c..5cdf6c7 100644
--- a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
@@ -413,6 +413,34 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
     end
   end
 
+  test "reuse Job with Docker image repo+tag" do
+    filter_h = BASE_FILTERS.
+      merge("script_version" =>
+              ["=", "4fe459abe02d9b365932b8f5dc419439ab4e2577"],
+            "docker_image_locator" =>
+              ["in docker", links(:docker_image_collection_tag2).name])
+    post(:create, {
+           job: {
+             script: "hash",
+             script_version: "4fe459abe02d9b365932b8f5dc419439ab4e2577",
+             repository: "foo",
+             script_parameters: {
+               input: 'fa7aeb5140e2848d39b416daeef4ffc5+45',
+               an_integer: '1'
+             },
+           },
+           filters: filters_from_hash(filter_h),
+           find_or_create: true,
+         })
+    assert_response :success
+    new_job = assigns(:object)
+    assert_not_nil new_job
+    target_job = jobs(:previous_docker_job_run)
+    [:uuid, :script_version, :docker_image_locator].each do |attr|
+      assert_equal(target_job.send(attr), new_job.send(attr))
+    end
+  end
+
   test "new job with unknown Docker image filter" do
     filter_h = BASE_FILTERS.
       merge("docker_image_locator" => ["in docker", "_nonesuchname_"])

commit 784e112f3fcad487c9f359413fb0dcd8b09c3edb
Author: Brett Smith <brett at curoverse.com>
Date:   Tue Jun 17 12:23:32 2014 -0400

    2879: Use Job-specific filters in #index too.
    
    This brings #index consistency with the #create method, and enables
    clients to do the same kind of sophisticated searching.

diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index c8765c1..d823b00 100644
--- a/services/api/app/controllers/arvados/v1/jobs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/jobs_controller.rb
@@ -47,44 +47,13 @@ class Arvados::V1::JobsController < ApplicationController
         else
           @filters.append(["docker_image_locator", "=", nil])
         end
-      else
-        script_info = {"repository" => nil, "script" => nil}
-        script_range = Hash.new { |key| [] }
-        @filters.select! do |filter|
-          if script_info.has_key? filter[0]
-            script_info[filter[0]] = filter[2]
-          end
-          case filter[0..1]
-          when ["script_version", "in git"]
-            script_range["min_version"] = filter.last
-            false
-          when ["script_version", "not in"], ["script_version", "not in git"]
-            begin
-              script_range["exclude_versions"] += filter.last
-            rescue TypeError
-              script_range["exclude_versions"] << filter.last
-            end
-            false
-          when ["docker_image_locator", "in docker"], ["docker_image_locator", "not in docker"]
-            filter[1].sub!(/ docker$/, '')
-            filter[2] = Collection.uuids_for_docker_image(filter[2])
-            true
-          else
-            true
+      else  # Check specified filters for some reasonableness.
+        filter_names = @filters.map { |f| f.first }.uniq
+        ["repository", "script"].each do |req_filter|
+          if not filter_names.include?(req_filter)
+            raise ArgumentError.new("#{req_filter} filter required")
           end
         end
-
-        script_info.each_pair do |key, value|
-          raise ArgumentError.new("#{key} filter required") if value.nil?
-        end
-        unless script_range.empty?
-          @filters.append(["script_version", "in",
-                           Commit.find_commit_range(current_user,
-                                                    script_info["repository"],
-                                                    script_range["min_version"],
-                                                    resource_attrs[:script_version],
-                                                    script_range["exclude_versions"])])
-        end
       end
 
       # Search for a reusable Job, and return it if found.
@@ -192,4 +161,56 @@ class Arvados::V1::JobsController < ApplicationController
   def self._queue_requires_parameters
     self._index_requires_parameters
   end
+
+  protected
+
+  def load_filters_param
+    # Convert Job-specific git and Docker filters into normal SQL filters.
+    super
+    script_info = {"repository" => nil, "script" => nil}
+    script_range = {"exclude_versions" => []}
+    @filters.select! do |filter|
+      if (script_info.has_key? filter[0]) and (filter[1] == "=")
+        if script_info[filter[0]].nil?
+          script_info[filter[0]] = filter[2]
+        elsif script_info[filter[0]] != filter[2]
+          raise ArgumentError.new("incompatible #{filter[0]} filters")
+        end
+      end
+      case filter[0..1]
+      when ["script_version", "in git"]
+        script_range["min_version"] = filter.last
+        false
+      when ["script_version", "not in git"]
+        begin
+          script_range["exclude_versions"] += filter.last
+        rescue TypeError
+          script_range["exclude_versions"] << filter.last
+        end
+        false
+      when ["docker_image_locator", "in docker"], ["docker_image_locator", "not in docker"]
+        filter[1].sub!(/ docker$/, '')
+        filter[2] = Collection.uuids_for_docker_image(filter[2])
+        true
+      else
+        true
+      end
+    end
+
+    # Build a real script_version filter from any "not? in git" filters.
+    if (script_range.size > 1) or script_range["exclude_versions"].any?
+      script_info.each_pair do |key, value|
+        if value.nil?
+          raise ArgumentError.new("script_version filter needs #{key} filter")
+        end
+      end
+      last_version = begin resource_attrs[:script_version] rescue "HEAD" end
+      @filters.append(["script_version", "in",
+                       Commit.find_commit_range(current_user,
+                                                script_info["repository"],
+                                                script_range["min_version"],
+                                                last_version,
+                                                script_range["exclude_versions"])])
+    end
+  end
 end
diff --git a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
index 0a61b0f..450093c 100644
--- a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
@@ -455,4 +455,35 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
                       "bad status code with missing #{skip_key} filter")
     end
   end
+
+  test "find Job with script version range" do
+    get :index, filters: [["repository", "=", "foo"],
+                          ["script", "=", "hash"],
+                          ["script_version", "in git", "tag1"]]
+    assert_response :success
+    assert_not_nil assigns(:objects)
+    assert_includes(assigns(:objects).map { |job| job.uuid },
+                    jobs(:previous_job_run).uuid)
+  end
+
+  test "find Job with script version range exclusions" do
+    get :index, filters: [["repository", "=", "foo"],
+                          ["script", "=", "hash"],
+                          ["script_version", "not in git", "tag1"]]
+    assert_response :success
+    assert_not_nil assigns(:objects)
+    refute_includes(assigns(:objects).map { |job| job.uuid },
+                    jobs(:previous_job_run).uuid)
+  end
+
+  test "find Job with Docker image range" do
+    get :index, filters: [["docker_image_locator", "in docker",
+                           "arvados/apitestfixture"]]
+    assert_response :success
+    assert_not_nil assigns(:objects)
+    assert_includes(assigns(:objects).map { |job| job.uuid },
+                    jobs(:previous_docker_job_run).uuid)
+    refute_includes(assigns(:objects).map { |job| job.uuid },
+                    jobs(:previous_job_run).uuid)
+  end
 end

commit 613773f364adb36de6b061ba5a78a6df0a53df4f
Author: Brett Smith <brett at curoverse.com>
Date:   Thu Jun 19 13:24:27 2014 -0400

    2879: API server can find_or_create Jobs based on Docker image.

diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index 6fddba7..c8765c1 100644
--- a/services/api/app/controllers/arvados/v1/jobs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/jobs_controller.rb
@@ -13,7 +13,6 @@ class Arvados::V1::JobsController < ApplicationController
         }, status: :unprocessable_entity
       end
     end
-    load_filters_param
 
     # We used to ask for the minimum_, exclude_, and no_reuse params
     # in the job resource. Now we advertise them as flags that alter
@@ -28,43 +27,67 @@ class Arvados::V1::JobsController < ApplicationController
     end
 
     if params[:find_or_create]
-      # Convert old special-purpose creation parameters to the new
-      # filters-based method.
-      minimum_script_version = params[:minimum_script_version]
-      exclude_script_versions = params.fetch(:exclude_script_versions, [])
-      @filters.select do |(col_name, operand, operator)|
-        case col_name
-        when "script_version"
-          case operand
-          when "in range"
-            minimum_script_version = operator
+      load_filters_param
+      if @filters.empty?  # Translate older creation parameters into filters.
+        @filters = [:repository, :script].map do |attrsym|
+          [attrsym.to_s, "=", resource_attrs[attrsym]]
+        end
+        @filters.append(["script_version", "in",
+                         Commit.find_commit_range(current_user,
+                                                  resource_attrs[:repository],
+                                                  params[:minimum_script_version],
+                                                  resource_attrs[:script_version],
+                                                  params[:exclude_script_versions])])
+        if image_search = resource_attrs[:runtime_constraints].andand["docker_image"]
+          image_tag = resource_attrs[:runtime_constraints]["docker_image_tag"]
+          image_locator = Collection.
+            uuids_for_docker_image(image_search, image_tag, @read_users).first
+          return super if image_locator.nil?  # We won't find anything to reuse.
+          @filters.append(["docker_image_locator", "=", image_locator])
+        else
+          @filters.append(["docker_image_locator", "=", nil])
+        end
+      else
+        script_info = {"repository" => nil, "script" => nil}
+        script_range = Hash.new { |key| [] }
+        @filters.select! do |filter|
+          if script_info.has_key? filter[0]
+            script_info[filter[0]] = filter[2]
+          end
+          case filter[0..1]
+          when ["script_version", "in git"]
+            script_range["min_version"] = filter.last
             false
-          when "not in", "not in range"
+          when ["script_version", "not in"], ["script_version", "not in git"]
             begin
-              exclude_script_versions += operator
+              script_range["exclude_versions"] += filter.last
             rescue TypeError
-              exclude_script_versions << operator
+              script_range["exclude_versions"] << filter.last
             end
             false
+          when ["docker_image_locator", "in docker"], ["docker_image_locator", "not in docker"]
+            filter[1].sub!(/ docker$/, '')
+            filter[2] = Collection.uuids_for_docker_image(filter[2])
+            true
           else
             true
           end
-        else
-          true
         end
-      end
-      @filters.append(["script_version", "in",
-                       Commit.find_commit_range(current_user,
-                                                resource_attrs[:repository],
-                                                minimum_script_version,
-                                                resource_attrs[:script_version],
-                                                exclude_script_versions)])
 
-      # Set up default filters for specific parameters.
-      if @filters.select { |f| f.first == "script" }.empty?
-        @filters.append(["script", "=", resource_attrs[:script]])
+        script_info.each_pair do |key, value|
+          raise ArgumentError.new("#{key} filter required") if value.nil?
+        end
+        unless script_range.empty?
+          @filters.append(["script_version", "in",
+                           Commit.find_commit_range(current_user,
+                                                    script_info["repository"],
+                                                    script_range["min_version"],
+                                                    resource_attrs[:script_version],
+                                                    script_range["exclude_versions"])])
+        end
       end
 
+      # Search for a reusable Job, and return it if found.
       @objects = Job.readable_by(current_user)
       apply_filters
       @object = nil
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 64a6bb0..e9be28b 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -148,7 +148,7 @@ class Collection < ArvadosModel
     [hash_part, size_part].compact.join '+'
   end
 
-  def self.for_latest_docker_image(search_term, search_tag=nil, readers=nil)
+  def self.uuids_for_docker_image(search_term, search_tag=nil, readers=nil)
     readers ||= [Thread.current[:user]]
     base_search = Link.
       readable_by(*readers).
@@ -161,7 +161,7 @@ class Collection < ArvadosModel
     coll_matches = base_search.
       where(link_class: "docker_image_hash", collections: {uuid: search_term})
     if match = coll_matches.first
-      return find_by_uuid(match.head_uuid)
+      return [match.head_uuid]
     end
 
     # Find Collections with matching Docker image repository+tag pairs.
@@ -176,20 +176,24 @@ class Collection < ArvadosModel
               "docker_image_hash", "#{search_term}%")
     end
 
-    # Select the image that was created most recently.  Note that the
-    # SQL search order and fallback timestamp values are chosen so
-    # that if image timestamps are missing, we use the image with the
-    # newest link.
-    latest_image_link = nil
-    latest_image_timestamp = "1900-01-01T00:00:00Z"
+    # Generate an order key for each result.  We want to order the results
+    # so that anything with an image timestamp is considered more recent than
+    # anything without; then we use the link's created_at as a tiebreaker.
+    uuid_timestamps = {}
     matches.find_each do |link|
-      link_timestamp = link.properties.fetch("image_timestamp",
-                                             "1900-01-01T00:00:01Z")
-      if link_timestamp > latest_image_timestamp
-        latest_image_link = link
-        latest_image_timestamp = link_timestamp
-      end
+      uuid_timestamps[link.head_uuid] =
+        [(-link.properties["image_timestamp"].to_datetime.to_i rescue 0),
+         -link.created_at.to_i]
+    end
+    uuid_timestamps.keys.sort_by { |uuid| uuid_timestamps[uuid] }
+  end
+
+  def self.for_latest_docker_image(search_term, search_tag=nil, readers=nil)
+    image_uuid = uuids_for_docker_image(search_term, search_tag, readers).first
+    if image_uuid.nil?
+      nil
+    else
+      find_by_uuid(image_uuid)
     end
-    latest_image_link.nil? ? nil : find_by_uuid(latest_image_link.head_uuid)
   end
 end
diff --git a/services/api/test/fixtures/jobs.yml b/services/api/test/fixtures/jobs.yml
index 3ad7746..adfc90f 100644
--- a/services/api/test/fixtures/jobs.yml
+++ b/services/api/test/fixtures/jobs.yml
@@ -117,6 +117,7 @@ barbaz:
 previous_job_run:
   uuid: zzzzz-8i9sb-cjs4pklxxjykqqq
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  repository: foo
   script: hash
   script_version: 4fe459abe02d9b365932b8f5dc419439ab4e2577
   script_parameters:
@@ -125,9 +126,23 @@ previous_job_run:
   success: true
   output: ea10d51bcf88862dbcc36eb292017dfd+45
 
+previous_docker_job_run:
+  uuid: zzzzz-8i9sb-k6emstgk4kw4yhi
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  repository: foo
+  script: hash
+  script_version: 4fe459abe02d9b365932b8f5dc419439ab4e2577
+  script_parameters:
+    input: fa7aeb5140e2848d39b416daeef4ffc5+45
+    an_integer: "1"
+  success: true
+  output: ea10d51bcf88862dbcc36eb292017dfd+45
+  docker_image_locator: fa3c1a9cb6783f85f2ecda037e07b8c3+167
+
 previous_job_run_no_output:
   uuid: zzzzz-8i9sb-cjs4pklxxjykppp
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  repository: foo
   script: hash
   script_version: 4fe459abe02d9b365932b8f5dc419439ab4e2577
   script_parameters:
@@ -139,6 +154,7 @@ previous_job_run_no_output:
 nondeterminisic_job_run:
   uuid: zzzzz-8i9sb-cjs4pklxxjykyyy
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  repository: foo
   script: hash2
   script_version: 4fe459abe02d9b365932b8f5dc419439ab4e2577
   script_parameters:
diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index ae2511b..b35e7d4 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -468,7 +468,7 @@ docker_image_collection_hash:
   tail_uuid: ~
   head_uuid: fa3c1a9cb6783f85f2ecda037e07b8c3+167
   properties:
-    image_timestamp: 2014-06-10T14:30:00.184019565Z
+    image_timestamp: "2014-06-10T14:30:00.184019565Z"
 
 docker_image_collection_repository:
   uuid: zzzzz-o0j2j-dockercollrepos
@@ -483,7 +483,7 @@ docker_image_collection_repository:
   tail_uuid: ~
   head_uuid: fa3c1a9cb6783f85f2ecda037e07b8c3+167
   properties:
-    image_timestamp: 2014-06-10T14:30:00.184019565Z
+    image_timestamp: "2014-06-10T14:30:00.184019565Z"
 
 docker_image_collection_tag:
   uuid: zzzzz-o0j2j-dockercolltagbb
@@ -498,7 +498,7 @@ docker_image_collection_tag:
   tail_uuid: ~
   head_uuid: fa3c1a9cb6783f85f2ecda037e07b8c3+167
   properties:
-    image_timestamp: 2014-06-10T14:30:00.184019565Z
+    image_timestamp: "2014-06-10T14:30:00.184019565Z"
 
 docker_image_collection_tag2:
   uuid: zzzzz-o0j2j-dockercolltagbc
@@ -513,7 +513,7 @@ docker_image_collection_tag2:
   tail_uuid: ~
   head_uuid: fa3c1a9cb6783f85f2ecda037e07b8c3+167
   properties:
-    image_timestamp: 2014-06-10T14:30:00.184019565Z
+    image_timestamp: "2014-06-10T14:30:00.184019565Z"
 
 ancient_docker_image_collection_hash:
   # This image helps test that searches for Docker images find
@@ -532,4 +532,4 @@ ancient_docker_image_collection_hash:
   tail_uuid: ~
   head_uuid: b519d9cb706a29fc7ea24dbea2f05851+249025
   properties:
-    image_timestamp: 2010-06-10T14:30:00.184019565Z
+    image_timestamp: "2010-06-10T14:30:00.184019565Z"
diff --git a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
index 363c468..0a61b0f 100644
--- a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
@@ -278,7 +278,20 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
     assert_equal '077ba2ad3ea24a929091a9e6ce545c93199b8e57', new_job['script_version']
   end
 
+  BASE_FILTERS = {
+    'repository' => ['=', 'foo'],
+    'script' => ['=', 'hash'],
+    'script_version' => ['in git', 'master'],
+    'docker_image_locator' => ['=', nil],
+  }
+
+  def filters_from_hash(hash)
+    hash.each_pair.map { |name, filter| [name] + filter }
+  end
+
   test "can reuse a Job based on filters" do
+    filter_h = BASE_FILTERS.
+      merge('script_version' => ['in git', 'tag1'])
     post(:create, {
            job: {
              script: "hash",
@@ -289,7 +302,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
                an_integer: '1'
              }
            },
-           filters: [["script_version", "in range", "tag1"]],
+           filters: filters_from_hash(filter_h),
            find_or_create: true,
          })
     assert_response :success
@@ -300,6 +313,10 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
   end
 
   test "can not reuse a Job based on filters" do
+    filter_a = filters_from_hash(BASE_FILTERS.reject { |k| k == 'script_version' })
+    filter_a += [["script_version", "in git",
+                  "31ce37fe365b3dc204300a3e4c396ad333ed0556"],
+                 ["script_version", "not in git", ["tag1"]]]
     post(:create, {
            job: {
              script: "hash",
@@ -310,9 +327,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
                an_integer: '1'
              }
            },
-           filters: [["script_version", "in range",
-                      "31ce37fe365b3dc204300a3e4c396ad333ed0556"],
-                     ["script_version", "not in", ["tag1"]]],
+           filters: filter_a,
            find_or_create: true,
          })
     assert_response :success
@@ -323,6 +338,8 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
   end
 
   test "can not reuse a Job based on arbitrary filters" do
+    filter_h = BASE_FILTERS.
+      merge("created_at" => ["<", "2010-01-01T00:00:00Z"])
     post(:create, {
            job: {
              script: "hash",
@@ -333,7 +350,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
                an_integer: '1'
              }
            },
-           filters: [["created_at", "<", "2010-01-01T00:00:00Z"]],
+           filters: filters_from_hash(filter_h),
            find_or_create: true,
          })
     assert_response :success
@@ -342,4 +359,100 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
     assert_not_equal 'zzzzz-8i9sb-cjs4pklxxjykqqq', new_job['uuid']
     assert_equal '4fe459abe02d9b365932b8f5dc419439ab4e2577', new_job['script_version']
   end
+
+  test "can reuse a Job with a Docker image" do
+    post(:create, {
+           job: {
+             script: "hash",
+             script_version: "4fe459abe02d9b365932b8f5dc419439ab4e2577",
+             repository: "foo",
+             script_parameters: {
+               input: 'fa7aeb5140e2848d39b416daeef4ffc5+45',
+               an_integer: '1'
+             },
+             runtime_constraints: {
+               docker_image: 'arvados/apitestfixture',
+             }
+           },
+           find_or_create: true,
+         })
+    assert_response :success
+    new_job = assigns(:object)
+    assert_not_nil new_job
+    target_job = jobs(:previous_docker_job_run)
+    [:uuid, :script_version, :docker_image_locator].each do |attr|
+      assert_equal(target_job.send(attr), new_job.send(attr))
+    end
+  end
+
+  test "can reuse a Job with a Docker image hash filter" do
+    filter_h = BASE_FILTERS.
+      merge("script_version" =>
+              ["=", "4fe459abe02d9b365932b8f5dc419439ab4e2577"],
+            "docker_image_locator" =>
+              ["in docker", links(:docker_image_collection_hash).name])
+    post(:create, {
+           job: {
+             script: "hash",
+             script_version: "4fe459abe02d9b365932b8f5dc419439ab4e2577",
+             repository: "foo",
+             script_parameters: {
+               input: 'fa7aeb5140e2848d39b416daeef4ffc5+45',
+               an_integer: '1'
+             },
+           },
+           filters: filters_from_hash(filter_h),
+           find_or_create: true,
+         })
+    assert_response :success
+    new_job = assigns(:object)
+    assert_not_nil new_job
+    target_job = jobs(:previous_docker_job_run)
+    [:uuid, :script_version, :docker_image_locator].each do |attr|
+      assert_equal(target_job.send(attr), new_job.send(attr))
+    end
+  end
+
+  test "new job with unknown Docker image filter" do
+    filter_h = BASE_FILTERS.
+      merge("docker_image_locator" => ["in docker", "_nonesuchname_"])
+    post(:create, {
+           job: {
+             script: "hash",
+             script_version: "4fe459abe02d9b365932b8f5dc419439ab4e2577",
+             repository: "foo",
+             script_parameters: {
+               input: 'fa7aeb5140e2848d39b416daeef4ffc5+45',
+               an_integer: '1'
+             },
+           },
+           filters: filters_from_hash(filter_h),
+           find_or_create: true,
+         })
+    assert_response :success
+    new_job = assigns(:object)
+    assert_not_nil new_job
+    assert_not_equal(jobs(:previous_docker_job_run).uuid, new_job.uuid)
+  end
+
+  ["repository", "script"].each do |skip_key|
+    test "missing #{skip_key} filter raises an error" do
+      filter_a = filters_from_hash(BASE_FILTERS.reject { |k| k == skip_key })
+      post(:create, {
+             job: {
+               script: "hash",
+               script_version: "master",
+               repository: "foo",
+               script_parameters: {
+                 input: 'fa7aeb5140e2848d39b416daeef4ffc5+45',
+                 an_integer: '1'
+               }
+             },
+             filters: filter_a,
+             find_or_create: true,
+           })
+      assert_includes(405..599, @response.code.to_i,
+                      "bad status code with missing #{skip_key} filter")
+    end
+  end
 end

commit 8c9b4a1c7b4ad3feb5c0ccb14888aad7ed262e18
Author: Brett Smith <brett at curoverse.com>
Date:   Mon Jun 16 16:49:15 2014 -0400

    2879: API server can find_or_create Jobs based on filters.
    
    This will make it easier to support new criteria for find_or_create.
    In particular, this will be the basis for searching based on Docker
    image.

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 9a54abe..fe5598e 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -130,14 +130,17 @@ class ApplicationController < ActionController::Base
     apply_where_limit_order_params
   end
 
-  def apply_where_limit_order_params
-    ar_table_name = @objects.table_name
-
-    ft = record_filters @filters, ar_table_name
+  def apply_filters
+    ft = record_filters @filters, @objects.table_name
     if ft[:cond_out].any?
       @objects = @objects.where(ft[:cond_out].join(' AND '), *ft[:param_out])
     end
+  end
 
+  def apply_where_limit_order_params
+    apply_filters
+
+    ar_table_name = @objects.table_name
     if @where.is_a? Hash and @where.any?
       conditions = ['1=1']
       @where.each do |attr,value|
diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index ffa78b9..6fddba7 100644
--- a/services/api/app/controllers/arvados/v1/jobs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/jobs_controller.rb
@@ -13,6 +13,7 @@ class Arvados::V1::JobsController < ApplicationController
         }, status: :unprocessable_entity
       end
     end
+    load_filters_param
 
     # We used to ask for the minimum_, exclude_, and no_reuse params
     # in the job resource. Now we advertise them as flags that alter
@@ -27,18 +28,48 @@ class Arvados::V1::JobsController < ApplicationController
     end
 
     if params[:find_or_create]
-      r = Commit.find_commit_range(current_user,
-                                   resource_attrs[:repository],
-                                   params[:minimum_script_version],
-                                   resource_attrs[:script_version],
-                                   params[:exclude_script_versions])
-      # Search for jobs whose script_version is in the list of commits
-      # returned by find_commit_range
+      # Convert old special-purpose creation parameters to the new
+      # filters-based method.
+      minimum_script_version = params[:minimum_script_version]
+      exclude_script_versions = params.fetch(:exclude_script_versions, [])
+      @filters.select do |(col_name, operand, operator)|
+        case col_name
+        when "script_version"
+          case operand
+          when "in range"
+            minimum_script_version = operator
+            false
+          when "not in", "not in range"
+            begin
+              exclude_script_versions += operator
+            rescue TypeError
+              exclude_script_versions << operator
+            end
+            false
+          else
+            true
+          end
+        else
+          true
+        end
+      end
+      @filters.append(["script_version", "in",
+                       Commit.find_commit_range(current_user,
+                                                resource_attrs[:repository],
+                                                minimum_script_version,
+                                                resource_attrs[:script_version],
+                                                exclude_script_versions)])
+
+      # Set up default filters for specific parameters.
+      if @filters.select { |f| f.first == "script" }.empty?
+        @filters.append(["script", "=", resource_attrs[:script]])
+      end
+
+      @objects = Job.readable_by(current_user)
+      apply_filters
       @object = nil
       incomplete_job = nil
-      Job.readable_by(current_user).where(script: resource_attrs[:script],
-                                          script_version: r).
-        each do |j|
+      @objects.each do |j|
         if j.nondeterministic != true and
             ((j.success == true and j.output != nil) or j.running == true) and
             j.script_parameters == resource_attrs[:script_parameters]
diff --git a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
index bfecf54..363c468 100644
--- a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
@@ -278,4 +278,68 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
     assert_equal '077ba2ad3ea24a929091a9e6ce545c93199b8e57', new_job['script_version']
   end
 
+  test "can reuse a Job based on filters" do
+    post(:create, {
+           job: {
+             script: "hash",
+             script_version: "master",
+             repository: "foo",
+             script_parameters: {
+               input: 'fa7aeb5140e2848d39b416daeef4ffc5+45',
+               an_integer: '1'
+             }
+           },
+           filters: [["script_version", "in range", "tag1"]],
+           find_or_create: true,
+         })
+    assert_response :success
+    assert_not_nil assigns(:object)
+    new_job = JSON.parse(@response.body)
+    assert_equal 'zzzzz-8i9sb-cjs4pklxxjykqqq', new_job['uuid']
+    assert_equal '4fe459abe02d9b365932b8f5dc419439ab4e2577', new_job['script_version']
+  end
+
+  test "can not reuse a Job based on filters" do
+    post(:create, {
+           job: {
+             script: "hash",
+             script_version: "master",
+             repository: "foo",
+             script_parameters: {
+               input: 'fa7aeb5140e2848d39b416daeef4ffc5+45',
+               an_integer: '1'
+             }
+           },
+           filters: [["script_version", "in range",
+                      "31ce37fe365b3dc204300a3e4c396ad333ed0556"],
+                     ["script_version", "not in", ["tag1"]]],
+           find_or_create: true,
+         })
+    assert_response :success
+    assert_not_nil assigns(:object)
+    new_job = JSON.parse(@response.body)
+    assert_not_equal 'zzzzz-8i9sb-cjs4pklxxjykqqq', new_job['uuid']
+    assert_equal '077ba2ad3ea24a929091a9e6ce545c93199b8e57', new_job['script_version']
+  end
+
+  test "can not reuse a Job based on arbitrary filters" do
+    post(:create, {
+           job: {
+             script: "hash",
+             script_version: "4fe459abe02d9b365932b8f5dc419439ab4e2577",
+             repository: "foo",
+             script_parameters: {
+               input: 'fa7aeb5140e2848d39b416daeef4ffc5+45',
+               an_integer: '1'
+             }
+           },
+           filters: [["created_at", "<", "2010-01-01T00:00:00Z"]],
+           find_or_create: true,
+         })
+    assert_response :success
+    assert_not_nil assigns(:object)
+    new_job = JSON.parse(@response.body)
+    assert_not_equal 'zzzzz-8i9sb-cjs4pklxxjykqqq', new_job['uuid']
+    assert_equal '4fe459abe02d9b365932b8f5dc419439ab4e2577', new_job['script_version']
+  end
 end

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list