[#561] implementation: get_maximum_permissions_for_path by d-w-moore · Pull Request #602 · irods/python-irodsclient

d-w-moore · 2024-08-02T04:59:33Z

Check if the user named (may be a user or group) has privilege at or above the given access_level to the collection or data object named by path

d-w-moore · 2024-08-02T04:59:44Z

Needs test.

d-w-moore · 2024-12-02T19:38:45Z

Looks as if we won't implement this, at least not in this way. close?

alanking · 2024-12-02T19:43:36Z

I vote we leave it open / in draft until an alternative implementation presents itself. Is there a reason you want to close this?

d-w-moore · 2024-12-03T05:06:59Z

I vote we leave it open / in draft until an alternative implementation presents itself. Is there a reason you want to close this?

No reason. I think we decided it's best implemented as a specific query.

korydraughn · 2024-12-03T13:24:39Z

We need to do more investigative work for #561.

Let's keep this in draft for now. We can circle back once the 3.0.0 milestone is complete.

d-w-moore · 2026-10-25T17:49:13Z

Hm. Looks like I force-pushed over the old branch. Note for the future, don't do that. (Look for pre-existence of the branch name in GitHub. Duh.)

Well, luckily I am very confident of the new code being better....

d-w-moore · 2026-10-25T17:50:59Z

Will mark ready for review once I've run the tests through with rudra's changes included

A query is done to determine the highest permission level the given user (or group) has on the target collection/data_object. A specific query will be used if the calling user has the availability, otherwise a reasonably efficient general query is used instead.

trel · 2026-10-26T17:36:02Z

+
+            # Find out if the query is already registered.
+            try:
+                specific_query = SpecificQuery(ses, alias='findQueryByAlias', args=('getMaximumPermissionForPath',))


i think we've leaned into snake case...

Suggested change

specific_query = SpecificQuery(ses, alias='findQueryByAlias', args=('getMaximumPermissionForPath',))

specific_query = SpecificQuery(ses, alias='findQueryByAlias', args=('get_maximum_permission_for_path',))

i think we've leaned into snake case...

maybe. It will be the first one using snake case, I guess.

i think we've done it before. certainly for other policy work that needed them...

Noting here that the SpecificQueries used by the logical quotas REP use snake case. I like snake case, but I don't feel strongly about it either way.

korydraughn · 2026-10-26T17:33:51Z

+  SQL='''
+WITH target_user AS (
+    SELECT user_id
+    FROM r_user_main
+    WHERE user_name = ?
+      AND zone_name = ?
+),
+resolved_object AS (
+    -- The path names a collection


Is there a reason this specific query isn't part of irods/irods rather than the PRC?

It will be. Meanwhile, things work whether or not there is a pre-packaged query. Baby steps I think - as we also haven't yet discussed and definitively decided an alias name for this query.

Is there a reason this specific query isn't part of irods/irods rather than the PRC?

if it's going to work regardless of whether it's already in the server... then the client code doesn't have to define the specific query... right?

That's correct. We typically do not make clients add SpecificQueries even if it's possible.

I'm down with that.

korydraughn · 2026-10-26T17:37:18Z

+            # Find out if the query is already registered.
+            try:
+                specific_query = SpecificQuery(ses, alias='findQueryByAlias', args=('getMaximumPermissionForPath',))
+                specific_query.execute()
+            except CAT_NO_ROWS_FOUND:
+                specific_query = None
+            else:
+                specific_query = SpecificQuery(ses, alias='getUserAccessType', args=specific_query_args)
+
+            # If there is no canned query available, and we are a rodsadmin, then install our own:
+            if not specific_query and ses.users.get(ses.username).type == 'rodsadmin':
+                specific_query = UserAccessTypeQuery(ses, args=specific_query_args)


Auto-installing a specific query is not how we've done things in the past.

We should just try to use it and if it's there, we're good. If not, use the fallback implementation. No need to test for existence.

but this is better for testing, in the mean time, so it might be better to leave it there until an iRODS release containing the new specific query.

If it's for test purposes, have the test explicitly add/remove the SpecificQuery. It's fine if the test tests for existence of the SpecificQuery before adding/removing it.

korydraughn · 2026-10-26T17:38:13Z


 class AccessManager(Manager):
+
+    def get_maximum_permission_for_path(self,


Missing documentation.

korydraughn · 2026-10-26T17:42:09Z

+        coll=None,
+        data=None,


Consider using more letters for the parameter names.

coll is pretty obvious, but data isn't.

korydraughn · 2026-10-26T17:42:38Z

+        force_genquery=False,
+        feedback=None,
+        return_type = irods.access.return_type.INTEGER


What do these parameters mean/do?

force_genquery - use genquery regardless of the availability or permissions for using a specific query (good for testing, at present)

feedback - gives caller the method of query used. ('general', 'specific')

return_type - determines whether eg. 'own' or 1200 is returned.

Are feedback and return_type meant for users or testing? Can you explain when someone might use those?

feedback is of utility for thorough testing , use is only important if the calling application is interested which query type was used. Otherwise we could replace it with logging.

return_type converts the integer to a string such as modify_object or own before returning. I don't know if this is preferable to letting the application do the conversion instead. Thoughts?

feedback is of utility for thorough testing , use is only important if the calling application is interested which query type was used. Otherwise we could replace it with logging.

If it's purely for testing, logging or thorough manual testing is sufficient. You can leave a big fat comment in the code for the next developer to be careful not to break the implementation.

return_type converts the integer to a string such as modify_object or own before returning. I don't know if this is preferable to letting the application do the conversion instead. Thoughts?

Hmm. If the SpecificQuery returns both the symbolic name and integer, then we can return a tuple containing them. Otherwise, I think we should pick one. If we have to pick one, I think the symbolic name is slightly stronger since it's used through out our docs and APIs.

korydraughn · 2026-10-26T17:46:53Z

+class return_type(enum.Enum):
+    INTEGER = 1
+    STRING = 2


Needs documentation.

korydraughn · 2026-10-26T17:48:44Z

        return response

-    def execute(self, limit=MAX_SQL_ROWS, offset=0, options=0, conditions=None):
+    def execute(self, limit=MAX_SQL_ROWS, offset=0, options=0, conditions=None, instance=None):


What does instance do?

executes from an alternative instance of the class, other than self. I will review this and make a more complete answer later. Ideally this shouldn't have been necessary.

d-w-moore marked this pull request as draft August 2, 2024 04:59

user equality operator (for group membership test)

f93a93c

d-w-moore force-pushed the 561.m branch from 0298c5f to 70401a0 Compare October 25, 2026 17:44

d-w-moore self-assigned this Oct 25, 2026

d-w-moore changed the title ~~[#561] experimental implementation: user_has_access~~ [#561] implementation: get_maximum_permissions_for_path Oct 25, 2026

d-w-moore added 2 commits October 26, 2026 11:50

[_561] test is now passing, yay

66676ca

d-w-moore force-pushed the 561.m branch from 480300b to 66676ca Compare October 26, 2026 15:50

d-w-moore added 2 commits October 26, 2026 13:08

whitespace

36bba51

appropriate(?) name for registered specific query

7744d93

d-w-moore marked this pull request as ready for review October 26, 2026 17:12

trel reviewed Oct 26, 2026

View reviewed changes

korydraughn reviewed Oct 26, 2026

View reviewed changes

	specific_query = SpecificQuery(ses, alias='findQueryByAlias', args=('getMaximumPermissionForPath',))
	specific_query = SpecificQuery(ses, alias='findQueryByAlias', args=('get_maximum_permission_for_path',))


		class AccessManager(Manager):

		def get_maximum_permission_for_path(self,

Conversation

d-w-moore commented Aug 2, 2024

Uh oh!

d-w-moore commented Aug 2, 2024

Uh oh!

d-w-moore commented Dec 2, 2024

Uh oh!

alanking commented Dec 2, 2024

Uh oh!

d-w-moore commented Dec 3, 2024

Uh oh!

korydraughn commented Dec 3, 2024

Uh oh!

d-w-moore commented Oct 25, 2026

Uh oh!

d-w-moore commented Oct 25, 2026

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 Oct 27, 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!

korydraughn Oct 27, 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d-w-moore Oct 27, 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!

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

d-w-moore Oct 27, 2026 •

edited

Loading

korydraughn Oct 27, 2026 •

edited

Loading

d-w-moore Oct 27, 2026 •

edited

Loading