Bundle of fixes (main) #110
Conversation
korydraughn
force-pushed
the
bundle.m
branch
2 times, most recently
from
070f178
to
78c2d42
Compare
|
After some discussion, we've decided to drop the commit containing Original work can be found here: https://github.com/korydraughn/irods_rule_engine_plugin_logical_quotas/tree/return_violation_on_matching_total_max_data_size.m |
|
The whole nested shared monitored collections thing was added because the plugin uses the RsComm as-is to update quotas. If the user has permission to write to a collection under someone else's collection, updating the quota of the collection will work. However, when the plugin starts walking up the logical path to update quotas on parent-parent collections, that user may NOT have permission to update those which will lead to an error. This is entirely avoidable by creating a client-side connection as the service account and updating the quotas using the admin flag. Doing that will slow things down due to TCP connection setup/teardown, but it is guaranteed to avoid the issue described. Using a new connection also means the server can exhaust its resources if there's a lot of activity happening on the server. If use of a new client connection isn't ideal, then it's on the admin to manage the permissions and decide whether nested shared monitored collections should be allowed. Thoughts? |
|
Can we walk up the tree as-is, and then, if a permission error is hit, switch to adminFlag? So we get speed for normal cases, and then have a silent, successful fallback for the scenario you describe? |
|
That is possible. Just need to determine if that's a simple/quick tweak or not. |
|
It's a complex / not-obvious scenario. So, it can wait if there are other things, I think. |
|
Not a blocker for sure. It was something that crossed my mind. I'll put it in an issue so it's not lost. You agree with the additional wording in the README? |
Sure. This is good - and helps people think about ramifications of alternate approaches. |
|
All tests passed. Squashing. |
|
Squashed. |
|
Tweaked a test name. Will pound after the tests come back as passing. |
…t monitoring collections.
|
Pounded. |
|
Merging. |
All tests passed.
Have to add a test for the commit containing
(TEST THIS)in the commit message.