[ARVADOS] updated: 5c45c63e6a4acbbe6ec70f4e8deb7796df642ef5
git at public.curoverse.com
git at public.curoverse.com
Tue Jun 17 12:22:46 EDT 2014
Summary of changes:
.../app/controllers/arvados/v1/jobs_controller.rb | 105 ++++++++++++++-------
services/api/test/fixtures/jobs.yml | 4 +
.../arvados/v1/job_reuse_controller_test.rb | 91 ++++++++++++++++--
3 files changed, 156 insertions(+), 44 deletions(-)
via 5c45c63e6a4acbbe6ec70f4e8deb7796df642ef5 (commit)
via d9658f9649ffd27a4affef53e4a1ad1f9bef1809 (commit)
via c9cccd204fe60964a366a880592973e2af8d3459 (commit)
from 1f2a831876fe1fd193c27639078d2de43cfd691b (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 5c45c63e6a4acbbe6ec70f4e8deb7796df642ef5
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 b1b394d..20b809b 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 d9658f9649ffd27a4affef53e4a1ad1f9bef1809
Author: Brett Smith <brett at curoverse.com>
Date: Tue Jun 17 11:26:40 2014 -0400
2879: Don't implicitly build Job find_or_create filters.
If a client is using filters, they're new enough to know that they
need to be explicit about exactly what they want to filter. This will
hopefully make client development easier to debug. Per discussion
with Tom Clegg.
diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index 0f9fc74..b1b394d 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,42 +27,17 @@ class Arvados::V1::JobsController < ApplicationController
end
if params[:find_or_create]
- # Translate older creation parameters and special range operators
- # into standard filters.
- minimum_script_version = params[:minimum_script_version]
- exclude_script_versions = params.fetch(:exclude_script_versions, [])
- @filters.select do |filter|
- case filter[0..1]
- when ["script_version", "in git"]
- minimum_script_version = filter.last
- false
- when ["script_version", "not in"], ["script_version", "not in git"]
- begin
- exclude_script_versions += filter.last
- rescue TypeError
- exclude_script_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
+ 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
- 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
- if @filters.select { |f| f.first == "docker_image_locator" }.empty?
+ @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 =
@@ -73,6 +47,44 @@ 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
+ 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.
diff --git a/services/api/test/fixtures/jobs.yml b/services/api/test/fixtures/jobs.yml
index c9b2588..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:
@@ -128,6 +129,7 @@ previous_job_run:
previous_docker_job_run:
uuid: zzzzz-8i9sb-k6emstgk4kw4yhi
owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ repository: foo
script: hash
script_version: 4fe459abe02d9b365932b8f5dc419439ab4e2577
script_parameters:
@@ -140,6 +142,7 @@ previous_docker_job_run:
previous_job_run_no_output:
uuid: zzzzz-8i9sb-cjs4pklxxjykppp
owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ repository: foo
script: hash
script_version: 4fe459abe02d9b365932b8f5dc419439ab4e2577
script_parameters:
@@ -151,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/functional/arvados/v1/job_reuse_controller_test.rb b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
index f6711df..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 git", "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 git",
- "31ce37fe365b3dc204300a3e4c396ad333ed0556"],
- ["script_version", "not in git", ["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
@@ -369,6 +386,11 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
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",
@@ -379,8 +401,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
an_integer: '1'
},
},
- filters: [["docker_image_locator", "in docker",
- links(:docker_image_collection_hash).name]],
+ filters: filters_from_hash(filter_h),
find_or_create: true,
})
assert_response :success
@@ -393,6 +414,8 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
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",
@@ -403,7 +426,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
an_integer: '1'
},
},
- filters: [["docker_image_locator", "in docker", "_nonesuchname_"]],
+ filters: filters_from_hash(filter_h),
find_or_create: true,
})
assert_response :success
@@ -411,4 +434,25 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
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 c9cccd204fe60964a366a880592973e2af8d3459
Author: Brett Smith <brett at curoverse.com>
Date: Tue Jun 17 10:20:42 2014 -0400
2879: Rename Job-specific filter operators.
Based on discussion with Tom Clegg.
diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index a93167a..0f9fc74 100644
--- a/services/api/app/controllers/arvados/v1/jobs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/jobs_controller.rb
@@ -34,18 +34,18 @@ class Arvados::V1::JobsController < ApplicationController
exclude_script_versions = params.fetch(:exclude_script_versions, [])
@filters.select do |filter|
case filter[0..1]
- when ["script_version", "in range"]
+ when ["script_version", "in git"]
minimum_script_version = filter.last
false
- when ["script_version", "not in"], ["script_version", "not in range"]
+ when ["script_version", "not in"], ["script_version", "not in git"]
begin
exclude_script_versions += filter.last
rescue TypeError
exclude_script_versions << filter.last
end
false
- when ["docker_image_locator", "in range"], ["docker_image_locator", "not in range"]
- filter[1].sub!(/ range$/, '')
+ 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
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 e534adc..f6711df 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
@@ -289,7 +289,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
an_integer: '1'
}
},
- filters: [["script_version", "in range", "tag1"]],
+ filters: [["script_version", "in git", "tag1"]],
find_or_create: true,
})
assert_response :success
@@ -310,9 +310,9 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
an_integer: '1'
}
},
- filters: [["script_version", "in range",
+ filters: [["script_version", "in git",
"31ce37fe365b3dc204300a3e4c396ad333ed0556"],
- ["script_version", "not in", ["tag1"]]],
+ ["script_version", "not in git", ["tag1"]]],
find_or_create: true,
})
assert_response :success
@@ -379,7 +379,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
an_integer: '1'
},
},
- filters: [["docker_image_locator", "in range",
+ filters: [["docker_image_locator", "in docker",
links(:docker_image_collection_hash).name]],
find_or_create: true,
})
@@ -403,7 +403,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
an_integer: '1'
},
},
- filters: [["docker_image_locator", "in range", "_nonexistentname_"]],
+ filters: [["docker_image_locator", "in docker", "_nonesuchname_"]],
find_or_create: true,
})
assert_response :success
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list