[ARVADOS] created: 1.1.3-253-g378eeb1
Git user
git at public.curoverse.com
Thu Mar 29 15:31:24 EDT 2018
at 378eeb13b1c4d188d07bde0abfe3955ad53d4beb (commit)
commit 378eeb13b1c4d188d07bde0abfe3955ad53d4beb
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date: Thu Mar 29 16:25:12 2018 -0300
13168: Use helper to avoid updating 'modified_by_user_uuid' field on CRs.
When a container's state changes, the related container requests are updated
using the system user privileges, now these updates don't assign the system user's
uuid to the container requests 'modified_by_user_uuid' field.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 6a59d3b..b9edeae 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -2,6 +2,7 @@
#
# SPDX-License-Identifier: AGPL-3.0
+require 'arvados_model_updates'
require 'has_uuid'
require 'record_filters'
require 'serializers'
@@ -10,6 +11,7 @@ require 'request_error'
class ArvadosModel < ActiveRecord::Base
self.abstract_class = true
+ include ArvadosModelUpdates
include CurrentApiClient # current_user, current_api_client, etc.
include DbCurrentTime
extend RecordFilters
@@ -539,7 +541,9 @@ class ArvadosModel < ActiveRecord::Base
self.updated_at = current_time
self.owner_uuid ||= current_default_owner if self.respond_to? :owner_uuid=
self.modified_at = current_time
- self.modified_by_user_uuid = current_user ? current_user.uuid : nil
+ if !anonymous_updater
+ self.modified_by_user_uuid = current_user ? current_user.uuid : nil
+ end
self.modified_by_client_uuid = current_api_client ? current_api_client.uuid : nil
true
end
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 3765266..8882b2c 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -7,6 +7,7 @@ require 'whitelist_update'
require 'safe_json'
class Container < ArvadosModel
+ include ArvadosModelUpdates
include HasUuid
include KindAndEtag
include CommonApiTemplate
@@ -560,9 +561,11 @@ class Container < ArvadosModel
c = Container.create! c_attrs
retryable_requests.each do |cr|
cr.with_lock do
- # Use row locking because this increments container_count
- cr.container_uuid = c.uuid
- cr.save!
+ leave_modified_by_user_alone do
+ # Use row locking because this increments container_count
+ cr.container_uuid = c.uuid
+ cr.save!
+ end
end
end
end
@@ -570,7 +573,9 @@ class Container < ArvadosModel
# Notify container requests associated with this container
ContainerRequest.where(container_uuid: uuid,
state: ContainerRequest::Committed).each do |cr|
- cr.finalize!
+ leave_modified_by_user_alone do
+ cr.finalize!
+ end
end
# Cancel outstanding container requests made by this container.
@@ -578,19 +583,20 @@ class Container < ArvadosModel
includes(:container).
where(requesting_container_uuid: uuid,
state: ContainerRequest::Committed).each do |cr|
- cr.update_attributes!(priority: 0)
- cr.container.reload
- if cr.container.state == Container::Queued || cr.container.state == Container::Locked
- # If the child container hasn't started yet, finalize the
- # child CR now instead of leaving it "on hold", i.e.,
- # Queued with priority 0. (OTOH, if the child is already
- # running, leave it alone so it can get cancelled the
- # usual way, get a copy of the log collection, etc.)
- cr.update_attributes!(state: ContainerRequest::Final)
+ leave_modified_by_user_alone do
+ cr.update_attributes!(priority: 0)
+ cr.container.reload
+ if cr.container.state == Container::Queued || cr.container.state == Container::Locked
+ # If the child container hasn't started yet, finalize the
+ # child CR now instead of leaving it "on hold", i.e.,
+ # Queued with priority 0. (OTOH, if the child is already
+ # running, leave it alone so it can get cancelled the
+ # usual way, get a copy of the log collection, etc.)
+ cr.update_attributes!(state: ContainerRequest::Final)
+ end
end
end
end
end
end
-
end
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 3587cf0..bc01b33 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -5,6 +5,7 @@
require 'whitelist_update'
class ContainerRequest < ArvadosModel
+ include ArvadosModelUpdates
include HasUuid
include KindAndEtag
include CommonApiTemplate
@@ -110,7 +111,9 @@ class ContainerRequest < ArvadosModel
if state == Committed && Container.find_by_uuid(container_uuid).final?
reload
act_as_system_user do
- finalize!
+ leave_modified_by_user_alone do
+ finalize!
+ end
end
end
end
diff --git a/services/api/lib/arvados_model_updates.rb b/services/api/lib/arvados_model_updates.rb
new file mode 100644
index 0000000..b456bd3
--- /dev/null
+++ b/services/api/lib/arvados_model_updates.rb
@@ -0,0 +1,21 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+module ArvadosModelUpdates
+ # ArvadosModel checks this to decide whether it should update the
+ # 'modified_by_user_uuid' field.
+ def anonymous_updater
+ Thread.current[:anonymous_updater] || false
+ end
+
+ def leave_modified_by_user_alone
+ anonymous_updater_was = anonymous_updater
+ begin
+ Thread.current[:anonymous_updater] = true
+ yield
+ ensure
+ Thread.current[:anonymous_updater] = anonymous_updater_was
+ end
+ end
+end
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index 70ad11e..cc257cc 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -200,6 +200,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
test "Request is finalized when its container is cancelled" do
set_user_from_auth :active
cr = create_minimal_req!(priority: 1, state: "Committed", container_count_max: 1)
+ assert_equal users(:active).uuid, cr.modified_by_user_uuid
act_as_system_user do
Container.find_by_uuid(cr.container_uuid).
@@ -208,6 +209,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
cr.reload
assert_equal "Final", cr.state
+ assert_equal users(:active).uuid, cr.modified_by_user_uuid
end
test "Request is finalized when its container is completed" do
@@ -216,6 +218,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
cr = create_minimal_req!(owner_uuid: project.uuid,
priority: 1,
state: "Committed")
+ assert_equal users(:active).uuid, cr.modified_by_user_uuid
c = act_as_system_user do
c = Container.find_by_uuid(cr.container_uuid)
@@ -237,6 +240,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
cr.reload
assert_equal "Final", cr.state
+ assert_equal users(:active).uuid, cr.modified_by_user_uuid
['output', 'log'].each do |out_type|
pdh = Container.find_by_uuid(cr.container_uuid).send(out_type)
assert_equal(1, Collection.where(portable_data_hash: pdh,
@@ -261,6 +265,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
cr2 = create_minimal_req!
cr2.update_attributes!(priority: 10, state: "Committed", requesting_container_uuid: c.uuid, command: ["echo", "foo2"], container_count_max: 1)
cr2.reload
+ assert_equal users(:active).uuid, cr2.modified_by_user_uuid
c2 = Container.find_by_uuid cr2.container_uuid
assert_operator 0, :<, c2.priority
@@ -275,6 +280,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
cr2.reload
assert_equal 0, cr2.priority
+ assert_equal users(:active).uuid, cr2.modified_by_user_uuid
c2.reload
assert_equal 0, c2.priority
commit 98fb00994467e805f764d799e78420ecac9c0879
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date: Thu Mar 29 13:31:29 2018 -0300
13168: Remove commented code.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>
diff --git a/services/api/lib/current_api_client.rb b/services/api/lib/current_api_client.rb
index 711c663..4963867 100644
--- a/services/api/lib/current_api_client.rb
+++ b/services/api/lib/current_api_client.rb
@@ -134,18 +134,12 @@ module CurrentApiClient
end
def act_as_user user
- #auth_was = Thread.current[:api_client_authorization]
user_was = Thread.current[:user]
Thread.current[:user] = user
- #Thread.current[:api_client_authorization] = ApiClientAuthorization.
- # where('user_id=? and scopes is null', user.id).
- # order('expires_at desc').
- # first
begin
yield
ensure
Thread.current[:user] = user_was
- #Thread.current[:api_client_authorization] = auth_was
end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list