[arvados] updated: 2.7.0-6629-gce87d7b43c
git repository hosting
git at public.arvados.org
Fri May 31 03:08:03 UTC 2024
Summary of changes:
sdk/python/arvados/util.py | 26 +++++++++++++++++++++-----
sdk/python/tests/test_computed_permissions.py | 11 +++++++++++
services/api/app/models/computed_permission.rb | 4 ++++
services/api/lib/load_param.rb | 4 ++--
4 files changed, 38 insertions(+), 7 deletions(-)
via ce87d7b43cc2192a6ca13e3eaef10457c1715f7c (commit)
from df9503c818bb07f86669ef20f014647203d77d6f (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 ce87d7b43cc2192a6ca13e3eaef10457c1715f7c
Author: Tom Clegg <tom at curii.com>
Date: Thu May 30 23:07:52 2024 -0400
20640: Fix keyset paging for computed permissions.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/sdk/python/arvados/util.py b/sdk/python/arvados/util.py
index 050c67f68d..a765bb9bdd 100644
--- a/sdk/python/arvados/util.py
+++ b/sdk/python/arvados/util.py
@@ -238,18 +238,32 @@ def keyset_list_all(
# In cases where there's more than one record with the
# same order key, the result could include records we
# already saw in the last page. Skip them.
- if i["uuid"] in seen_prevpage:
+ seen_key = None
+ if "uuid" in i:
+ seen_key = i["uuid"]
+ elif "user_uuid" in i and "target_uuid" in i:
+ # If we are looking at computed_permissions, there is
+ # no uuid field. The primary key is a tuple.
+ seen_key = (i["user_uuid"], i["target_uuid"])
+ if seen_key in seen_prevpage:
continue
- seen_thispage.add(i["uuid"])
+ if seen_key is not None:
+ seen_thispage.add(seen_key)
yield i
firstitem = items["items"][0]
lastitem = items["items"][-1]
+ tiebreak_key = "uuid"
+ if "uuid" not in lastitem and "target_uuid" in lastitem:
+ # Special case for computed_permissions, where items do
+ # not have a uuid.
+ tiebreak_key = "target_uuid"
+
if firstitem[order_key] == lastitem[order_key]:
# Got a page where every item has the same order key.
- # Switch to using uuid for paging.
- nextpage = [[order_key, "=", lastitem[order_key]], ["uuid", ">" if ascending else "<", lastitem["uuid"]]]
+ # Switch to using tiebreak key for paging.
+ nextpage = [[order_key, "=", lastitem[order_key]], [tiebreak_key, ">" if ascending else "<", lastitem[tiebreak_key]]]
prev_page_all_same_order_key = True
else:
# Start from the last order key seen, but skip the last
@@ -258,7 +272,9 @@ def keyset_list_all(
# still likely we'll end up retrieving duplicate rows.
# That's handled by tracking the "seen" rows for each page
# so they can be skipped if they show up on the next page.
- nextpage = [[order_key, ">=" if ascending else "<=", lastitem[order_key]], ["uuid", "!=", lastitem["uuid"]]]
+ nextpage = [[order_key, ">=" if ascending else "<=", lastitem[order_key]]]
+ if tiebreak_key == "uuid":
+ nextpage += [[tiebreak_key, "!=", lastitem[tiebreak_key]]]
prev_page_all_same_order_key = False
def ca_certs_path(fallback: T=httplib2.CA_CERTS) -> Union[str, T]:
diff --git a/sdk/python/tests/test_computed_permissions.py b/sdk/python/tests/test_computed_permissions.py
index 7f0eee2014..73043e648e 100644
--- a/sdk/python/tests/test_computed_permissions.py
+++ b/sdk/python/tests/test_computed_permissions.py
@@ -3,6 +3,7 @@
# SPDX-License-Identifier: Apache-2.0
import arvados
+import arvados.util
from . import run_test_server
class ComputedPermissionTest(run_test_server.TestCaseWithServers):
@@ -16,3 +17,13 @@ class ComputedPermissionTest(run_test_server.TestCaseWithServers):
assert len(resp['items']) > 0
for item in resp['items']:
assert item['user_uuid'] == active_user_uuid
+
+ def test_keyset_list_all(self):
+ run_test_server.authorize_with('admin')
+ api_client = arvados.api('v1')
+ seen = {}
+ for item in arvados.util.keyset_list_all(api_client.computed_permissions().list, order_key='user_uuid'):
+ import sys
+ print(f"{item['user_uuid']} {item['target_uuid']}", file=sys.stderr)
+ assert (item['user_uuid'], item['target_uuid']) not in seen
+ seen[(item['user_uuid'], item['target_uuid'])] = True
diff --git a/services/api/app/models/computed_permission.rb b/services/api/app/models/computed_permission.rb
index af89eadf1f..c89860c48e 100644
--- a/services/api/app/models/computed_permission.rb
+++ b/services/api/app/models/computed_permission.rb
@@ -55,4 +55,8 @@ class ComputedPermission < ApplicationRecord
def self.serialized_attributes
{}
end
+
+ def self.unique_columns
+ []
+ end
end
diff --git a/services/api/lib/load_param.rb b/services/api/lib/load_param.rb
index 9a360c538b..29f3e35df5 100644
--- a/services/api/lib/load_param.rb
+++ b/services/api/lib/load_param.rb
@@ -93,14 +93,14 @@ module LoadParam
# The attr can have its table unspecified if it happens to be for the current "model_class" (the first case)
# or it can be fully specified with the database tablename (the second case) (e.g. "collections.name").
# NB that the security check for the second case table_name will not work if the model
- # has used set_table_name to use an alternate table name from the Rails standard.
+ # has used table_name= to use an alternate table name from the Rails standard.
# I could not find a perfect way to handle this well, but ActiveRecord::Base.send(:descendants)
# would be a place to start if this ever becomes necessary.
if (attr.match(/^[a-z][_a-z0-9]+$/) &&
model_class.columns.collect(&:name).index(attr) &&
['asc','desc'].index(direction.downcase))
if fill_table_names
- @orders << "#{table_name}.#{attr} #{direction.downcase}"
+ @orders << "#{model_class.table_name}.#{attr} #{direction.downcase}"
else
@orders << "#{attr} #{direction.downcase}"
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list