-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unnecessary performance penalty when checking list item unique permissions #2407
Comments
Hi @anghelnicolae - thanks for filing. In psuedo-code, what it does today is:
and I think you're suggesting that it could/should be:
The problem with that is that fetching site list items is done via the Microsoft Graph API, where fetching role assignments is done via the Sharepoint Rest API. And AFAIK, the Microsoft Graph API does not have the ability to fetch role assignments. You can see the list of available relationships and properties for List Items in the Graph API here: https://learn.microsoft.com/en-us/graph/api/resources/listitem?view=graph-rest-1.0 We use the Graph API as much as possible, because its rate limit quotas are more forgiving, and it's the Microsoft-recommended approach for interacting with Sharepoint where possible. We are unlikely to use the Sharepoint Rest API to retrieve list items, even if it would have this performance benefit here, because it would decrease performance for all users - even those not using Document Level Security. If you want to avoid this performance hit, you can choose not to fetch role assignments for the list items with the connector configuration of |
Hi @seanstory ,
|
Perhaps I'm misunderstanding your suggestion still. Can you share a sample curl for what you're suggesting, to fetch |
On line 894 it could be something like this:
|
@anghelnicolae no, I don't believe that will work. Like I said above, the Graph API doesn't have a property or relationship for
as soon as I add |
If Graph doesn't offer this property it is still better to use the REST API to get all the list items with unique permissions in one request than to test each item individually. |
Yeah, probably. But that's only the case if DLS is enabled. Perhaps what we could do is change the logic from (psudocode):
to
So effectively taking a totally different strategy for fetching list items, depending on if DLS is enabled or not 🤔 I think it's unlikely that we'll prioritize this immediately, but we can leave this open. If you have a support contract with Elastic, you can ask your account rep to file an Enhancement Request to help us understand any urgency on this, and prioritize accordingly. You're also welcome to take a stab at making a PR yourself, if you're not interested in waiting for us to get around to this. |
The property "HasUniqueRoleAssignments" can be added to the "$select" parameter when getting list items in bulk, instead of doing a request for each item.
connectors/connectors/sources/sharepoint_online.py
Line 1978 in 0d35433
The text was updated successfully, but these errors were encountered: