[ARVADOS] updated: 14b762d2dd37a0a533532839aaf0488c11205e42
git at public.curoverse.com
git at public.curoverse.com
Tue Nov 18 09:21:56 EST 2014
Summary of changes:
doc/api/methods/jobs.html.textile.liquid | 6 +-
.../app/controllers/arvados/v1/jobs_controller.rb | 84 +++++++++++++---------
services/api/test/fixtures/jobs.yml | 17 +++++
.../arvados/v1/job_reuse_controller_test.rb | 81 ++++++++++++++-------
services/api/test/unit/job_test.rb | 11 ++-
5 files changed, 136 insertions(+), 63 deletions(-)
via 14b762d2dd37a0a533532839aaf0488c11205e42 (commit)
via 1a520a700456325a8a7427d74fa11ed15b62a4b2 (commit)
via e82958936210b6bbc8cc04b237229fdf6fac295c (commit)
from 4805867bc1004ae9d755a6375d592748a7b29585 (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 14b762d2dd37a0a533532839aaf0488c11205e42
Merge: 4805867 1a520a7
Author: Brett Smith <brett at curoverse.com>
Date: Tue Nov 18 09:21:24 2014 -0500
Merge branch '4027-sdk-constraint-reuse-wip'
Refs #4027. Closes #4532.
commit 1a520a700456325a8a7427d74fa11ed15b62a4b2
Author: Brett Smith <brett at curoverse.com>
Date: Mon Nov 17 17:31:12 2014 -0500
4027: Test updating job's SDK version after it's already set.
diff --git a/services/api/test/unit/job_test.rb b/services/api/test/unit/job_test.rb
index 32f9e3e..65ca935 100644
--- a/services/api/test/unit/job_test.rb
+++ b/services/api/test/unit/job_test.rb
@@ -320,7 +320,7 @@ class JobTest < ActiveSupport::TestCase
end
end
- test "updating job with SDK version '#{search}'" do
+ test "updating job from no SDK to version '#{search}'" do
job = Job.create!(job_attrs)
assert_nil job.arvados_sdk_version
check_job_sdk_version(commit_hash) do
@@ -328,6 +328,15 @@ class JobTest < ActiveSupport::TestCase
job
end
end
+
+ test "updating job from SDK version 'master' to '#{search}'" do
+ job = Job.create!(job_attrs(sdk_constraint("master")))
+ assert_equal(SDK_MASTER, job.arvados_sdk_version)
+ check_job_sdk_version(commit_hash) do
+ job.runtime_constraints = sdk_constraint(search)[:runtime_constraints]
+ job
+ end
+ end
end
test "clear the SDK version" do
commit e82958936210b6bbc8cc04b237229fdf6fac295c
Author: Brett Smith <brett at curoverse.com>
Date: Fri Nov 14 13:57:33 2014 -0500
4027: API server considers arvados_sdk_version for Job reuse.
* Extend "in git" and "not in git" filters to the arvados_sdk_version
column.
* When no filters are specified, if the submitted job specifies an
arvados_sdk_version constraint, require reuse candidates to meet it.
* Document all this.
diff --git a/doc/api/methods/jobs.html.textile.liquid b/doc/api/methods/jobs.html.textile.liquid
index 539a4a0..ac68129 100644
--- a/doc/api/methods/jobs.html.textile.liquid
+++ b/doc/api/methods/jobs.html.textile.liquid
@@ -45,9 +45,9 @@ 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@ @in git@ @REFSPEC@, @arvados_sdk_version@ @in git@ @REFSPEC@<br>Resolve @REFSPEC@ to a list of Git commits, and match jobs with a @script_version@ or @arvados_sdk_version@ in that list. When creating a job and filtering @script_version@, the search will find commits between @REFSPEC@ and the submitted job's @script_version@; all other searches will find commits 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.
+* @script_version@ @not in git@ @REFSPEC@, @arvados_sdk_version@ @not in git@ @REFSPEC@<br>Resolve @REFSPEC@ to a list of Git commits, and match jobs with a @script_version@ or @arvados_sdk_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.
@@ -61,7 +61,7 @@ notextile. <div class="spaced-out">
# If @find_or_create@ is false or omitted, create a new job and skip the rest of these steps.
# 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 @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 submitted job includes an @arvados_sdk_version@ constraint, jobs must have an @arvados_sdk_version@ between that refspec and HEAD to be found.
# 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.
diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index e2bef88..bd6cbd0 100644
--- a/services/api/app/controllers/arvados/v1/jobs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/jobs_controller.rb
@@ -43,6 +43,9 @@ class Arvados::V1::JobsController < ApplicationController
else
@filters.append(["docker_image_locator", "=", nil])
end
+ if sdk_version = resource_attrs[:runtime_constraints].andand["arvados_sdk_version"]
+ @filters.append(["arvados_sdk_version", "in git", sdk_version])
+ end
begin
load_job_specific_filters
rescue ArgumentError => error
@@ -196,59 +199,70 @@ class Arvados::V1::JobsController < ApplicationController
def load_job_specific_filters
# Convert Job-specific @filters entries into general SQL filters.
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")
+ git_filters = Hash.new do |hash, key|
+ hash[key] = {"max_version" => "HEAD", "exclude_versions" => []}
+ end
+ @filters.select! do |(attr, operator, operand)|
+ if (script_info.has_key? attr) and (operator == "=")
+ if script_info[attr].nil?
+ script_info[attr] = operand
+ elsif script_info[attr] != operand
+ raise ArgumentError.new("incompatible #{attr} filters")
end
end
- case filter[0..1]
- when ["script_version", "in git"]
- script_range["min_version"] = filter.last
+ case operator
+ when "in git"
+ git_filters[attr]["min_version"] = operand
false
- when ["script_version", "not in git"]
- begin
- script_range["exclude_versions"] += filter.last
- rescue TypeError
- script_range["exclude_versions"] << filter.last
- end
+ when "not in git"
+ git_filters[attr]["exclude_versions"] += Array.wrap(operand)
false
- when ["docker_image_locator", "in docker"], ["docker_image_locator", "not in docker"]
- filter[1].sub!(/ docker$/, '')
- search_list = filter[2].is_a?(Enumerable) ? filter[2] : [filter[2]]
- filter[2] = search_list.flat_map do |search_term|
+ when "in docker", "not in docker"
+ image_hashes = Array.wrap(operand).flat_map do |search_term|
image_search, image_tag = search_term.split(':', 2)
- Collection.find_all_for_docker_image(image_search, image_tag, @read_users).map(&:portable_data_hash)
+ Collection.
+ find_all_for_docker_image(image_search, image_tag, @read_users).
+ map(&:portable_data_hash)
end
- true
+ @filters << [attr, operator.sub(/ docker$/, ""), image_hashes]
+ false
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")
+ git_filters.each_pair do |attr, filter|
+ case attr
+ when "script_version"
+ script_info.each_pair do |key, value|
+ if value.nil?
+ raise ArgumentError.new("script_version filter needs #{key} filter")
+ end
end
+ filter["repository"] = script_info["repository"]
+ begin
+ filter["max_version"] = resource_attrs[:script_version]
+ rescue
+ # Using HEAD, set earlier by the hash default, is fine.
+ end
+ when "arvados_sdk_version"
+ filter["repository"] = "arvados"
+ else
+ raise ArgumentError.new("unknown attribute for git filter: #{attr}")
end
- last_version = begin resource_attrs[:script_version] rescue "HEAD" end
version_range = Commit.find_commit_range(current_user,
- script_info["repository"],
- script_range["min_version"],
- last_version,
- script_range["exclude_versions"])
+ filter["repository"],
+ filter["min_version"],
+ filter["max_version"],
+ filter["exclude_versions"])
if version_range.nil?
raise ArgumentError.
- new(["error searching #{script_info['repository']} from",
- "'#{script_range['min_version']}' to '#{last_version}',",
- "excluding #{script_range['exclude_versions']}"].join(" "))
+ new("error searching #{filter['repository']} from " +
+ "'#{filter['min_version']}' to '#{filter['max_version']}', " +
+ "excluding #{filter['exclude_versions']}")
end
- @filters.append(["script_version", "in", version_range])
+ @filters.append([attr, "in", version_range])
end
end
diff --git a/services/api/test/fixtures/jobs.yml b/services/api/test/fixtures/jobs.yml
index 6721c12..888cb2a 100644
--- a/services/api/test/fixtures/jobs.yml
+++ b/services/api/test/fixtures/jobs.yml
@@ -183,6 +183,23 @@ previous_docker_job_run:
docker_image_locator: fa3c1a9cb6783f85f2ecda037e07b8c3+167
state: Complete
+previous_job_run_with_arvados_sdk_version:
+ uuid: zzzzz-8i9sb-eoo0321or2dw2jg
+ created_at: <%= 14.minute.ago.to_s(:db) %>
+ owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ repository: foo
+ script: hash
+ script_version: 31ce37fe365b3dc204300a3e4c396ad333ed0556
+ script_parameters:
+ input: fa7aeb5140e2848d39b416daeef4ffc5+45
+ an_integer: "1"
+ runtime_constraints:
+ arvados_sdk_version: commit2
+ arvados_sdk_version: 00634b2b8a492d6f121e3cf1d6587b821136a9a7
+ success: true
+ output: ea10d51bcf88862dbcc36eb292017dfd+45
+ state: Complete
+
previous_job_run_no_output:
uuid: zzzzz-8i9sb-cjs4pklxxjykppp
created_at: <%= 14.minute.ago.to_s(:db) %>
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 01f2b9c..a3eebf3 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
@@ -294,7 +294,8 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
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']
+ assert_not_equal('4fe459abe02d9b365932b8f5dc419439ab4e2577',
+ new_job['script_version'])
end
test "cannot reuse job with find_or_create but excluded version" do
@@ -316,7 +317,8 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
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']
+ assert_not_equal('4fe459abe02d9b365932b8f5dc419439ab4e2577',
+ new_job['script_version'])
end
BASE_FILTERS = {
@@ -324,6 +326,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
'script' => ['=', 'hash'],
'script_version' => ['in git', 'master'],
'docker_image_locator' => ['=', nil],
+ 'arvados_sdk_version' => ['=', nil],
}
def filters_from_hash(hash)
@@ -331,7 +334,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
end
test "can reuse a Job based on filters" do
- filter_h = BASE_FILTERS.
+ filters_hash = BASE_FILTERS.
merge('script_version' => ['in git', 'tag1'])
post(:create, {
job: {
@@ -343,7 +346,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
an_integer: '1'
}
},
- filters: filters_from_hash(filter_h),
+ filters: filters_from_hash(filters_hash),
find_or_create: true,
})
assert_response :success
@@ -354,10 +357,11 @@ 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"]]]
+ filters = filters_from_hash(BASE_FILTERS
+ .reject { |k| k == 'script_version' })
+ filters += [["script_version", "in git",
+ "31ce37fe365b3dc204300a3e4c396ad333ed0556"],
+ ["script_version", "not in git", ["tag1"]]]
post(:create, {
job: {
script: "hash",
@@ -368,7 +372,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
an_integer: '1'
}
},
- filters: filter_a,
+ filters: filters,
find_or_create: true,
})
assert_response :success
@@ -379,7 +383,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
end
test "can not reuse a Job based on arbitrary filters" do
- filter_h = BASE_FILTERS.
+ filters_hash = BASE_FILTERS.
merge("created_at" => ["<", "2010-01-01T00:00:00Z"])
post(:create, {
job: {
@@ -391,7 +395,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
an_integer: '1'
}
},
- filters: filters_from_hash(filter_h),
+ filters: filters_from_hash(filters_hash),
find_or_create: true,
})
assert_response :success
@@ -427,7 +431,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
end
test "can reuse a Job with a Docker image hash filter" do
- filter_h = BASE_FILTERS.
+ filters_hash = BASE_FILTERS.
merge("script_version" =>
["=", "4fe459abe02d9b365932b8f5dc419439ab4e2577"],
"docker_image_locator" =>
@@ -442,7 +446,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
an_integer: '1'
},
},
- filters: filters_from_hash(filter_h),
+ filters: filters_from_hash(filters_hash),
find_or_create: true,
})
assert_response :success
@@ -455,7 +459,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
end
test "reuse Job with Docker image repo+tag" do
- filter_h = BASE_FILTERS.
+ filters_hash = BASE_FILTERS.
merge("script_version" =>
["=", "4fe459abe02d9b365932b8f5dc419439ab4e2577"],
"docker_image_locator" =>
@@ -470,7 +474,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
an_integer: '1'
},
},
- filters: filters_from_hash(filter_h),
+ filters: filters_from_hash(filters_hash),
find_or_create: true,
})
assert_response :success
@@ -483,7 +487,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
end
test "new job with unknown Docker image filter" do
- filter_h = BASE_FILTERS.
+ filters_hash = BASE_FILTERS.
merge("docker_image_locator" => ["in docker", "_nonesuchname_"])
post(:create, {
job: {
@@ -495,7 +499,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
an_integer: '1'
},
},
- filters: filters_from_hash(filter_h),
+ filters: filters_from_hash(filters_hash),
find_or_create: true,
})
assert_response :success
@@ -506,7 +510,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
["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 })
+ filters = filters_from_hash(BASE_FILTERS.reject { |k| k == skip_key })
post(:create, {
job: {
script: "hash",
@@ -517,7 +521,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
an_integer: '1'
}
},
- filters: filter_a,
+ filters: filters,
find_or_create: true,
})
assert_includes(405..599, @response.code.to_i,
@@ -632,8 +636,8 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
# 1de84a8 is on the b1 branch, after master's tip.
test "new job created from unsatisfiable minimum version filter" do
- filter_h = BASE_FILTERS.merge("script_version" => ["in git", "1de84a8"])
- check_new_job_created_from(filters: filters_from_hash(filter_h))
+ filters_hash = BASE_FILTERS.merge("script_version" => ["in git", "1de84a8"])
+ check_new_job_created_from(filters: filters_from_hash(filters_hash))
end
test "new job created from unsatisfiable minimum version parameter" do
@@ -645,9 +649,9 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
end
test "graceful error from nonexistent minimum version filter" do
- filter_h = BASE_FILTERS.merge("script_version" =>
- ["in git", "__nosuchbranch__"])
- errors = check_errors_from(filters: filters_from_hash(filter_h))
+ filters_hash = BASE_FILTERS.merge("script_version" =>
+ ["in git", "__nosuchbranch__"])
+ errors = check_errors_from(filters: filters_from_hash(filters_hash))
assert(errors.any? { |msg| msg.include? "__nosuchbranch__" },
"bad refspec not mentioned in error message")
end
@@ -663,4 +667,33 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
assert(errors.any? { |msg| msg.include? "__nosuchbranch__" },
"bad refspec not mentioned in error message")
end
+
+ test "can't reuse job with older Arvados SDK version" do
+ params = {
+ script_version: "31ce37fe365b3dc204300a3e4c396ad333ed0556",
+ runtime_constraints: {"arvados_sdk_version" => "master"},
+ }
+ check_new_job_created_from(job: params)
+ end
+
+ test "reuse job from arvados_sdk_version git filters" do
+ filters_hash = BASE_FILTERS.
+ merge("arvados_sdk_version" => ["in git", "commit2"])
+ filters_hash.delete("script_version")
+ params = create_foo_hash_job_params(filters:
+ filters_from_hash(filters_hash))
+ post(:create, params)
+ assert_response :success
+ assert_equal(jobs(:previous_job_run_with_arvados_sdk_version).uuid,
+ assigns(:object).uuid)
+ end
+
+ test "create new job because of arvados_sdk_version 'not in git' filters" do
+ filters_hash = BASE_FILTERS.reject { |k| k == "script_version" }
+ filters = filters_from_hash(filters_hash)
+ # Allow anything from the root commit, but before commit 2.
+ filters += [["arvados_sdk_version", "in git", "436637c8"],
+ ["arvados_sdk_version", "not in git", "00634b2b"]]
+ check_new_job_created_from(filters: filters)
+ end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list