irods / irods_rule_engine_plugin_metadata_guard

BSD 3-Clause "New" or "Revised" License
0 stars 7 forks source link

Atomic avu handling (main) #65

Closed FifthPotato closed 8 months ago

FifthPotato commented 8 months ago

Primary code mostly appears to work as expected, but can't get the test to stick. Calling it through the microservice seems to bypass policy (at least, I think) and I'm not sure if we can use the python client in tests. At a bit of a loss on how to properly test it with just icommands...

trel commented 8 months ago

fair. i don't think icommands have a binary that hits this API endpoint. we expose it via PRC... and yes, the microservice is already 'in the server', so wouldn't hit a PEP.

trel commented 8 months ago

and perhaps a run through clang-format? i see some long lines.

korydraughn commented 8 months ago

We're going to have to provide another mechanism for testing the atomic PEP. Creating a dependency on the PRC seems like the easiest path forward.

trel commented 8 months ago

agreed, for now.

alanking commented 8 months ago

Is this in draft because we are waiting on something? Please mark as ready for review when ready. Thanks

trel commented 8 months ago

please mark conversations as resolved/done when they are resolved/done. it's so close...

FifthPotato commented 8 months ago

I think I got it all. Did I miss anything?

korydraughn commented 8 months ago

Have you run all the plugin tests (i.e. the new ones and the old ones) to see if everything is passing?

If not, please do so and post a comment letting us know if they passed or failed. We'll do one more pass over the PR while that's happening.

FifthPotato commented 8 months ago

Yep, I've run them, and both old&new pass. This will also potentially require PRC to be added to the testing environment dockerfiles, methinks.

alanking commented 8 months ago

This will also potentially require PRC to be added to the testing environment dockerfiles, methinks.

Please create an issue for this and maybe link to this PR: https://github.com/irods/irods_testing_environment Thanks!

korydraughn commented 8 months ago

Can the test hook install the PRC? Is it weird to install python modules from a python script?

@alanking Who launches the test hook in the testing environment? root or irods?

alanking commented 8 months ago

Yeah, root runs the test hook in the testing environment now. A user with sudo access should run the test hook as they frequently install packages using the package manager and configure/run external services.

We install Minio packages via pip in the test hook for the S3 resource plugin. Not sure if it's weird, but there is precedent.

korydraughn commented 8 months ago

Good.

@FifthPotato Let's start with making the test hook install the PRC. I imagine it shouldn't take no more than one or two lines of code.

korydraughn commented 8 months ago

You could include issue 42 as well since it's marked as a duplicate of 38?

korydraughn commented 8 months ago

Please add an empty line between the main commit message and the lines that act as the body. For example:

[38,42] Primary commit message / Summary.

Body.
FifthPotato commented 8 months ago

Okay, messed with it until the commit msg doesn't get cut off, and put the rest into the body. Numbers added, too.

FifthPotato commented 8 months ago

Octothorpe'd