irods / irods_capability_automated_ingest

Other
12 stars 16 forks source link

`irods_sync` script fixes and enhancements (plus one other bug fix) #294

Closed alanking closed 2 weeks ago

alanking commented 2 weeks ago

Addresses #91 Addresses #92 Addresses #93 Addresses #210 Addresses #293

I'm contemplating adding tests for these subcommands, but it seems a little tricky (hence, draft PR) and there are no existing tests for any of the subcommands other than start. Existing tests are passing.

Also, the two commits referencing #210 (only) can be squashed into one if we like the solution of maintaining a list of stopped jobs. Otherwise, we can drop the second commit because the first one addresses the issue (albeit, not as well, IMO).

korydraughn commented 2 weeks ago

Are you looking for feedback on this PR even though it's in draft?

alanking commented 2 weeks ago

I want to confirm that this is doing what we want before marking as ready for review. Feel free to leave feedback on what you see, but the implementation may change significantly depending on how the requirements settle out.

korydraughn commented 2 weeks ago

Ok. Will put eyes on this once it's out of draft.

alanking commented 2 weeks ago

I squashed the two #210 commits together since we're pretty into the stopped jobs list. The stopped jobs list now includes all the information shown in the active jobs list subcommand output. I think this is ready for review...

korydraughn commented 2 weeks ago

Ok. Will get eyes on it.

korydraughn commented 2 weeks ago

Anything left for this? Are the tests passing?

alanking commented 2 weeks ago

I'll run the test suite once more for good measure, but these changes mainly affect the sync script rather than the framework, so these should probably be passing. I'll report back with results soon.

I would also like to update the commit message for "Reset Redis handles for stopped jobs", but that's about it for this one.

korydraughn commented 2 weeks ago

Sounds good. Will wait for the test results before approving.

alanking commented 2 weeks ago

Tests are passing.

alanking commented 2 weeks ago

Squashed, tweaked commit message, and added a thing that Black formatter caught.

alanking commented 2 weeks ago

'd, mergin