[#505] create an atomic ACLs endpoint by d-w-moore · Pull Request #808 · irods/python-irodsclient

d-w-moore · 2026-03-20T23:27:52Z

No test yet, will undraft when I have one.
Exercised now with this script:

# run as rods after 
# $ irm -fr b; itouch b
# $ iadmin mkzone twilight remote
# $ iadmin mkuser dan rodsuser
# $ iadmin mkuser charlie#twilight rodsuser

import irods
from irods.access import iRODSAccess
from irods.exception import iRODSException

s = irods.helpers.make_session()

try:
  pass
  s.acls._call_atomic_acl_api(
   "/tempZone/home/rods/b",
   iRODSAccess("write","","dan"),
   iRODSAccess("write","","public"),
   iRODSAccess("read","","charlie","twilight"),
 )
except iRODSException as exc:
  print(f'{exc!r}')
  e = exc
  print (e.server_msg.get_json_encoded_struct())
#except Exception as exc:
#  print(repr(exc))
else:
  print ('---> success')
  pass

d-w-moore · 2026-03-23T15:47:37Z

How would the team feel about my deprecating _iRODSAccess_pre_4_3_0, since we're now officially not supporting iRODS < 4.3.0 in PRC?

korydraughn · 2026-03-23T15:50:25Z

The leading underscore indicates that symbol is private to the implementation. If that's the case, no objections here.

korydraughn · 2026-03-23T15:51:08Z

Then again, if it's an implementation detail, then we don't have to deprecate it. We can just remove it.

d-w-moore · 2026-03-23T17:47:12Z

Ready for review. ACLOperation now created as part of an improved interface for atomic ACLs call.

korydraughn · 2026-03-23T23:19:06Z

Spotted this in the type checking workflow.

irods/manager/access_manager.py:54: error: Incompatible types in assignment (expression has type "list[Any]", target has type "str")  [assignment]

korydraughn

Looking good so far.

irods/manager/access_manager.py

korydraughn · 2026-03-23T23:25:50Z

irods/access.py

+    def __lt__(self, other):
+        return (
+            self.access_name < other.access_name
+            and self.user_name < other.user_name
+            and self.user_zone < other.user_zone
+        ) and iRODSPath(self.path) < iRODSPath(other.path)


How does this affect the Access class?
Do users need to know about this?

Thanks for drawing my attention to it, as it was improperly defined.

Yes, it maybe should have a comment at least.

It mainly just gives the class a defined sorting order. Now, if someone attempts pretty-printing a mixed list of these items, they'll get something the eye is somewhat able to follow, although I guess if the two formats were more consistent and the permissions were in order it could be better.... Currently though , you can see they are sortable and end up alphabetical by permission, with the path being last in sort order and not significant for items not having a 'path' member.

>>> from pprint import pp; from irods.access import iRODSAccess,ACLOperation >>> pp(sorted([ACLOperation('read_metadata','dan'),iRODSAccess('read_metadata','/t','bob'),ACLOperation('read','alice'), iRODSAccess('write','/t','alice'),ACLOperation('own','rods')])) [<ACLOperation: access_name='own' user_name='rods' user_zone=''>, <ACLOperation: access_name='read' user_name='alice' user_zone=''>, <iRODSAccess read_metadata /t bob >, <ACLOperation: access_name='read_metadata' user_name='dan' user_zone=''>, <iRODSAccess write /t alice >]

korydraughn · 2026-03-23T23:26:46Z

irods/access.py

        return hash((self.access_name, iRODSPath(self.path), self.user_name, self.user_zone))

-    def copy(self, decanonicalize=False):
+    def copy(self, decanonicalize=False, implied_zone=''):


What is implied_zone?
Do we expect users of the PRC to ever use this parameter?

It's useful for comparison purposes if you can tell __eq__ that a null length zone field implies the current zone name.

korydraughn · 2026-03-23T23:28:25Z

irods/access.py

        return f"<iRODSAccess {access_name} {self.path} {self.user_name}{user_type_hint} {self.user_zone}>"


+class ACLOperation(iRODSAccess):


What is the reason behind inheriting from iRODSAccess?
Are there benefits to tying these types together?

The benefits are basically that we can test equality and therefore in operator should work when mixing types up and down the inheritance hierarchy. So:

ACLOperation('own','bob') in [iRODSAccess('own','/my/object/path','bob')]

is True. This is leveraged in the test to affirm that the ACLOperations just atomically set are included in the list of iRODSAccess objects returned by session.acls.get.

I think the only other benefit is code reuse.

korydraughn · 2026-03-23T23:30:08Z

irods/test/access_test.py

+            user2 = ses.users.create("rod_serling_505#twilight", "rodsuser")
+            user3 = ses.users.create("local_test_user_505", "rodsuser")
+            group = ses.groups.create("test_group_505")
+            ses.acls._call_atomic_acl_api(


Is _call_atomic_acl_api a temporary thing?

I ask because of the leading underscore and use of call.

It seems like this is more or less based on the design for applying atomic metadata. Is that right?

That would make the _call_atomic_acl_api method make sense. Why is there no user-facing method, though? For atomic metadata, we have _call_atomic_metadata_api, but there is also apply_atomic_operations for client consumption. Shouldn't there be an apply_atomic_operations for the AccessManager in order for this to be analogous to the MetadataManager?

You are right. I think just a rename will be enough, so _call_atomic_metadata_api should become apply_atomic_operations.

atomic_apply_acl_operations so that it aligns with the API name?

that's fine. a little bit redundant maybe since it'll be called as:

session.acls.atomic_apply_acl_operations

but I think that's ok.

korydraughn · 2026-03-23T23:31:03Z

irods/test/access_test.py

+                a4:=ACLOperation("read", group.name),
+            )
+
+            normalize = lambda access: access.copy(decanonicalize=True, implied_zone=ses.zone)


What does this line do?

This is used to "flatten" iRODSAccess and ACLOperations to render common values into the appropriate fields to satisfy eq , ie. allow them to be "de facto equal" for test's purposes when they should be considered equivalent. THus we can effectively satisfy, for instance:

normalize( iRODSAccess('read', '/some/path','bob') )== normalize( ACLOperation('read_object','bob','tempZone'))

Co-authored-by: Kory Draughn <korydraughn@ymail.com>

irods/manager/access_manager.py

alanking · 2026-03-24T12:59:26Z

irods/test/access_test.py

+            user2 = ses.users.create("rod_serling_505#twilight", "rodsuser")
+            user3 = ses.users.create("local_test_user_505", "rodsuser")
+            group = ses.groups.create("test_group_505")
+            ses.acls._call_atomic_acl_api(


It seems like this is more or less based on the design for applying atomic metadata. Is that right?

That would make the _call_atomic_acl_api method make sense. Why is there no user-facing method, though? For atomic metadata, we have _call_atomic_metadata_api, but there is also apply_atomic_operations for client consumption. Shouldn't there be an apply_atomic_operations for the AccessManager in order for this to be analogous to the MetadataManager?

irods/manager/access_manager.py

d-w-moore · 2026-03-24T05:11:30Z

irods/access.py

+    def __lt__(self, other):
+        return (
+            self.access_name < other.access_name
+            and self.user_name < other.user_name
+            and self.user_zone < other.user_zone
+        ) and iRODSPath(self.path) < iRODSPath(other.path)


Thanks for drawing my attention to it, as it was improperly defined.

Yes, it maybe should have a comment at least.

It mainly just gives the class a defined sorting order. Now, if someone attempts pretty-printing a mixed list of these items, they'll get something the eye is somewhat able to follow, although I guess if the two formats were more consistent and the permissions were in order it could be better.... Currently though , you can see they are sortable and end up alphabetical by permission, with the path being last in sort order and not significant for items not having a 'path' member.

>>> from pprint import pp; from irods.access import iRODSAccess,ACLOperation >>> pp(sorted([ACLOperation('read_metadata','dan'),iRODSAccess('read_metadata','/t','bob'),ACLOperation('read','alice'), iRODSAccess('write','/t','alice'),ACLOperation('own','rods')])) [<ACLOperation: access_name='own' user_name='rods' user_zone=''>, <ACLOperation: access_name='read' user_name='alice' user_zone=''>, <iRODSAccess read_metadata /t bob >, <ACLOperation: access_name='read_metadata' user_name='dan' user_zone=''>, <iRODSAccess write /t alice >]

d-w-moore marked this pull request as draft March 20, 2026 23:28

d-w-moore added 2 commits March 21, 2026 00:37

add SYS_INTERNAL_ERR

7a81184

[_505,sq] atomic ACLs endpoint

228bf86

d-w-moore force-pushed the 505.m branch from 5a8986e to 228bf86 Compare March 21, 2026 04:42

ruff1

d277eb7

d-w-moore self-assigned this Mar 21, 2026

d-w-moore marked this pull request as ready for review March 21, 2026 04:45

d-w-moore added 2 commits March 23, 2026 13:39

[_505] create ACLOperation based on iRODSAccess

64b8701

[_505] test local users both with and without zone

1803dda

korydraughn reviewed Mar 23, 2026

View reviewed changes

Update irods/manager/access_manager.py

d419d0e

Co-authored-by: Kory Draughn <korydraughn@ymail.com>

alanking reviewed Mar 24, 2026

View reviewed changes

d-w-moore added 2 commits March 24, 2026 15:35

correct __lt__ comparison function

a746e2a

correction to dict literal

ef0fe69

d-w-moore commented Mar 24, 2026

View reviewed changes

d-w-moore added 2 commits March 24, 2026 16:02

api_number

b70413d

rename interface method

8a656a0

		return f"<iRODSAccess {access_name} {self.path} {self.user_name}{user_type_hint} {self.user_zone}>"


		class ACLOperation(iRODSAccess):

Conversation

d-w-moore commented Mar 20, 2026

Uh oh!

d-w-moore commented Mar 23, 2026

Uh oh!

korydraughn commented Mar 23, 2026

Uh oh!

korydraughn commented Mar 23, 2026

Uh oh!

d-w-moore commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

korydraughn commented Mar 23, 2026

Uh oh!

korydraughn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d-w-moore Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d-w-moore Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

d-w-moore commented Mar 23, 2026 •

edited

Loading

d-w-moore Mar 24, 2026 •

edited

Loading

d-w-moore Mar 24, 2026 •

edited

Loading