[ARVADOS] updated: bc47a4a09b43cda49a52d96d5dce17987813e43c
git at public.curoverse.com
git at public.curoverse.com
Tue Nov 18 09:00:25 EST 2014
Summary of changes:
.../app/controllers/arvados/v1/jobs_controller.rb | 41 +++++++-------
.../arvados/v1/job_reuse_controller_test.rb | 62 +++++++++++-----------
2 files changed, 51 insertions(+), 52 deletions(-)
via bc47a4a09b43cda49a52d96d5dce17987813e43c (commit)
from fc1a62883352b129ae9fc92efdc1bd7ef16972d7 (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 bc47a4a09b43cda49a52d96d5dce17987813e43c
Author: Brett Smith <brett at curoverse.com>
Date: Tue Nov 18 08:52:15 2014 -0500
4027: Fixups suggested by code review.
diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index 76528d0..bd6cbd0 100644
--- a/services/api/app/controllers/arvados/v1/jobs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/jobs_controller.rb
@@ -202,32 +202,29 @@ class Arvados::V1::JobsController < ApplicationController
git_filters = Hash.new do |hash, key|
hash[key] = {"max_version" => "HEAD", "exclude_versions" => []}
end
- @filters.select! do |(col, op, val)|
- if (script_info.has_key? col) and (op == "=")
- if script_info[col].nil?
- script_info[col] = val
- elsif script_info[col] != val
- raise ArgumentError.new("incompatible #{col} filters")
+ @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 op
+ case operator
when "in git"
- git_filters[col]["min_version"] = val
+ git_filters[attr]["min_version"] = operand
false
when "not in git"
- begin
- git_filters[col]["exclude_versions"] += val
- rescue TypeError
- git_filters[col]["exclude_versions"] << val
- end
+ git_filters[attr]["exclude_versions"] += Array.wrap(operand)
false
when "in docker", "not in docker"
- search_list = val.is_a?(Enumerable) ? val : [val]
- image_hashes = search_list.flat_map do |search_term|
+ 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
- @filters << [col, op.sub(/ docker$/, ""), image_hashes]
+ @filters << [attr, operator.sub(/ docker$/, ""), image_hashes]
false
else
true
@@ -235,8 +232,8 @@ class Arvados::V1::JobsController < ApplicationController
end
# Build a real script_version filter from any "not? in git" filters.
- git_filters.each_pair do |col, filter|
- case col
+ git_filters.each_pair do |attr, filter|
+ case attr
when "script_version"
script_info.each_pair do |key, value|
if value.nil?
@@ -247,12 +244,12 @@ class Arvados::V1::JobsController < ApplicationController
begin
filter["max_version"] = resource_attrs[:script_version]
rescue
- # Using the HEAD default is fine.
+ # Using HEAD, set earlier by the hash default, is fine.
end
when "arvados_sdk_version"
filter["repository"] = "arvados"
else
- raise ArgumentError.new("unknown column for git filter: #{col}")
+ raise ArgumentError.new("unknown attribute for git filter: #{attr}")
end
version_range = Commit.find_commit_range(current_user,
filter["repository"],
@@ -265,7 +262,7 @@ class Arvados::V1::JobsController < ApplicationController
"'#{filter['min_version']}' to '#{filter['max_version']}', " +
"excluding #{filter['exclude_versions']}")
end
- @filters.append([col, "in", version_range])
+ @filters.append([attr, "in", version_range])
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 d08fbbf..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
@@ -334,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: {
@@ -346,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
@@ -357,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",
@@ -371,7 +372,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
an_integer: '1'
}
},
- filters: filter_a,
+ filters: filters,
find_or_create: true,
})
assert_response :success
@@ -382,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: {
@@ -394,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
@@ -430,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" =>
@@ -445,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
@@ -458,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" =>
@@ -473,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
@@ -486,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: {
@@ -498,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
@@ -509,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",
@@ -520,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,
@@ -635,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
@@ -648,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
@@ -676,10 +677,11 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
end
test "reuse job from arvados_sdk_version git filters" do
- filter_h = BASE_FILTERS.
+ filters_hash = BASE_FILTERS.
merge("arvados_sdk_version" => ["in git", "commit2"])
- filter_h.delete("script_version")
- params = create_foo_hash_job_params(filters: filters_from_hash(filter_h))
+ 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,
@@ -687,11 +689,11 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
end
test "create new job because of arvados_sdk_version 'not in git' filters" do
- filter_h = BASE_FILTERS.reject { |k| k == "script_version" }
- filter_a = filters_from_hash(filter_h)
+ 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.
- filter_a += [["arvados_sdk_version", "in git", "436637c8"],
- ["arvados_sdk_version", "not in git", "00634b2b"]]
- check_new_job_created_from(filters: filter_a)
+ 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