Batching job-submit tasks to avoid blocking Stdout/err pipes

[wxtim]

Address part of #2857
Temporary PR to check fix on codacy.

[wxtim]

1. Does anyone have any thoughts on how (or whether) to add testing for this bug and solution?
2. I created a new function as a sub-function of taskJobManager.submit_task_jobs called list_chopper. I'm not sure that it belongs here - is there a general utils module somewhere in the project?

[hjoliver]

Temporary PR to check fix on codacy.

Note you can get codacy to analyze your own fork. Also fine to put up a "work in progress" PR though (which we seem to be labeling with "WIP" now, following @kinow).

[hjoliver]

Does anyone have any thoughts on how (or whether) to add testing for this bug and solution?

Maybe add some debug output on the number of jobs submitted, e.g. "50 jobs submitted, batch 1/4", and search for that in test suite output.

Maybe also make batch size a site config item, so we can scale it down for test suites.

[hjoliver]

I created a new function as a sub-function ... is there a general utils module somewhere in the project?

No, but there's several more specific utility modules (with "util" in their filenames). We don't generally bother unless/until functions are used in multiple places.

[kinow]

(@hjoliver on WIP prefix, in GitLab WIP in the title locks the pull request (called merge request there). Quite handy, as one cannot merge by accident. Others have submitted a feature request already)

[hjoliver]

(@kinow - that is a cool feature of GitLab).

[wxtim]

Does anyone have any thoughts on how (or whether) to add testing for this bug and solution?

Maybe add some debug output on the number of jobs submitted, e.g. "50 jobs submitted, batch 1/4", and search for that in test suite output.

Maybe also make batch size a site config item, so we can scale it down for test suites.

We had a discussion about this: I wanted to make it a config item but other people were not so sure it was necessary.

[wxtim]

I created a new function as a sub-function ... is there a general utils module somewhere in the project?

No, but there's several more specific utility modules (with "util" in their filenames). We don't generally bother unless/until functions are used in multiple places.

I pretty much got this response from @oliver-sanders review - alongside the suggestion that I collapse it into a generator comprehension for brevity.

[wxtim]

Current State of Play with pull request:

1. I've moved the sorting of the itasks list outside the part of the code I've changed so that the list gets sorted before being chopped up.
2. I've changed the generator comprehension back into a list comprehension to facilitate using len(itasks_batchs).
3. I've added a message to the LOG for debug mode that can be used by a test to check that the chunking has worked correctly.
4. I've changed the size of the chunks back to 100. I have no idea why I changed it to 50.
5. I've added a simple test designed to ensure that the batching is done correctly. Thanks to @matthewrmshin for the suite used to test the issue.

I haven't made the batch size a config item. The consensus here seems to be that it isn't necessary. In addition, my experience of writing the test suggests that it may be a bad idea® to allow this to be set. Setting it very low can cause large overheads in terms of numbers of processes spawned.

[wxtim]

@matthewrmshin - If you are now happy with this I'll squash it down.

[matthewrmshin]

Looks OK. Please remove the debug echo from the test.

[oliver-sanders]

All good here.