irods / irods_rule_engine_plugin_logical_quotas

BSD 3-Clause "New" or "Revised" License
1 stars 9 forks source link

[#84] use admin flag to change AVUs when recalculating totals (4-2-stable) #85

Closed d-w-moore closed 1 year ago

d-w-moore commented 1 year ago

When plugin needs to modify AVU's that track quotas on behalf of a rodsuser (e.g. replica_close as part of istream write; see #84) some new logic will be required to make this work. Because setting metadata can mean redirection to the provider from the connected server, the definitive fix appears to involve the agent opening a client connection to self and this requires resolution of irods/irods#6782.

Same logic applies for running a general query as a rodsadmin; in this case, it was necessary to author a dedicated function to execute the query using the C API calls because irods::query is already included/linked but only in the server-enabled version.

korydraughn commented 1 year ago

That is correct.

d-w-moore commented 1 year ago

@korydraughn Tried the client conn for set_metadata but I'm getting this compile error:

$ make
Scanning dependencies of target irods_rule_engine_plugin-logical_quotas
[ 33%] Building CXX object CMakeFiles/irods_rule_engine_plugin-logical_quotas.dir/src/handler.cpp.o
/tmp/irods_rule_engine_plugin_logical_quotas/src/handler.cpp:599:21: error: no member named 'client' in namespace 'irods::experimental::filesystem'
                fs::client::set_metadata(fs::admin, clientComm, path, {attrs.total_number_of_data_objects(), objects.empty() ? "0" : objects});
                ~~~~^
1 error generated.
CMakeFiles/irods_rule_engine_plugin-logical_quotas.dir/build.make:75: recipe for target 'CMakeFiles/irods_rule_engine_plugin-logical_quotas.dir/src/handler.cpp.o' failed
korydraughn commented 1 year ago

What's left for this?

d-w-moore commented 1 year ago

Yes, looks like TODOs we added are done, will remove them.

d-w-moore commented 1 year ago

Test(s) yet to be written, starting with that now.

korydraughn commented 1 year ago

We need to apply similar changes to handlers which make calls to the following functions:

d-w-moore commented 1 year ago

@korydraughn Good point.

d-w-moore commented 1 year ago

One of those functions, unset_metadata_impl, calls get_monitored_collection_info, and that in turn has an irods::query using an rsComm_t&. Can we now do that as an rcComm_t& from a server plugin? You've adapted the filesystem calls along these lines. I wonder, do we need to do similarly now with the irods_query.hpp header?

korydraughn commented 1 year ago

We may have to do something similar for irods_query.hpp, but for now, let's use the C API directly for those cases where we need to make a GenQuery call using an RcComm. We'll deal with irods_query.hpp in 4.3.

d-w-moore commented 1 year ago

About to make an enhancement request in irods/irods (milestone should be >=4.3.1 ) relevant to the PR description above:

Same logic applies for running a general query as a rodsadmin; in this case, it was necessary to author a dedicated function to execute the query using the C API calls because irods::query is already included/linked but only in the server-enabled version.

korydraughn commented 1 year ago

Yes, >=4.3.1.

korydraughn commented 1 year ago

I see unresolved comments on this PR. Please mark them as resolved if you've handled them.

d-w-moore commented 1 year ago

done!

korydraughn commented 1 year ago

Is this ready for review?

d-w-moore commented 1 year ago

Is this ready for review?

Yes. Also: (1) Rebase done as discussed yesterday and (2) I've also run the tests on the cherry-pick to main, with everything passing in both branches.

d-w-moore commented 1 year ago

Tests still pass. All conversations appear resolved except for the one, will take care of that one today.

korydraughn commented 1 year ago

Do all tests pass?

d-w-moore commented 1 year ago

Do all tests pass?

All tests pass at the bench.

korydraughn commented 1 year ago

Good. Please squash to taste and we'll do one last pass over it.

d-w-moore commented 1 year ago

squashed. will squash in main later, after making the issue for the one failing test.

d-w-moore commented 1 year ago

# is added!