[#81] Index user and group permissions (4-2-stable)

[d-w-moore]

No description provided.

[trel]

resolve the comments once they're handled.

[trel]

point of order - there are no more unresolved comments on this...

so... thoughts on squashing?

[d-w-moore]

Definitely, one final test though. Expect green, but don't want any surprises.

[d-w-moore]

Squashed

[trel]

i see two commits - the first will be reverted/removed before this is ready to merge?

[d-w-moore]

The first commit is a way of gracefully handling a issue/bug introduced in 4.2.9 iRODS server . Unrelated, so I could move it to be its own pull request, which might make more sense.

[d-w-moore]

Hm, looking at it again: it doesn't do much that's useful either, that "revert me on resolution of irods/irods#6100". Just changes the information that's logged. Probably will just drop it then. I am not sure how to handle replication with regard to indexing.

[d-w-moore]

I remembered why the revert me... 6100 change was done , originally.

An attempt to irepl will (post-4.2.9) results in seemingly obtuse message such as the following , whenever the indexing plugin is configured:

vagrant@ubun18y:~/github/b$ irepl -R abc a
remote addresses: 127.0.0.1 ERROR: replUtil: repl error for /tempZone/home/rods/a, status = -130000 status = -130000 SYS_INVALID_INPUT_PARAM
Level 0: resc hier is null

Does that leave a bad taste in anyone's mouth? I don't really have an opinion about it. This particular commit merely added a little extra diagnostic to the printed-out message that warned that the new replica would not be indexed because (after 4.2.9) resc_hier will aways be null on replication.

[trel]

we should definitely NOT be printing that out. we should also not have anything to index after a replication - there's no new data to be indexed... so, indexing should ignore replication...? yes?

[d-w-moore]

There's the case of a full-text indexing operation that we'd need to perform on the target repl if, previous to the replication, no repl's had yet existed on indexable resources.

[d-w-moore]

But the indexing plugin has write-only access to what is in elasticsearch.... so, if we attacked that "problem case" right now, being that irods/irods#6100 is still an issue, I'd think the solution would be the following:

Whenever a replication happens, we query unconditionally for repls existing on any indexable resources, and do a full-text on it if the AVU annotations say to do so. That's the brute-force way, but it has the advantage that we don't need the resc_hier to be non-null.
Or we wait til issue 6100 is solved and do it a more clever way, if that exists.

[trel]

Ah, I see. Forgot about the per-resource indexing functionality.

[d-w-moore]

So maybe I make the printing out of the distracting error message a separate issue, and "silence" it for now? Maybe also include an automatic reindex of the data object upon replication if any repls are on indexable resources.

[trel]

So maybe I make the printing out of the distracting error message a separate issue, and "silence" it for now?

Hm, silencing seems good if it's not actually an error. More info to the user, only in the case that they need it.

Maybe also include an automatic reindex of the data object upon replication if any repls are on indexable resources.

want to avoid reindexing events that aren't necessary - so maybe we write/talk this out a bit more ... can we enumerate the relevant scenarios?

Several force-pushes since this review, so the requested changes may be addressed. Removing review til I can look again

[alanking]

irods/irods#6100 is all but resolved at this point. Can we try this again without the workaround?

[d-w-moore]

Looks like this got left in an indefinite state. Do I need to test again with 4-2-stable . Perhaps the other branches too.

[d-w-moore]

@alanking what was the workaround?

[alanking]

19a570c and d0b2a61 are both workarounds for irods/irods#6100. That issue has been resolved in main, 4-3-stable, and 4-2-stable. I wanted to make sure that it met the expectations of this plugin before closing.

I guess you can revert the first or both of those commits and see what happens. No real rush on it, just wanted to leave a note.