Flux executor: Remove mini from submit command
#5229
Conversation
Signed-off-by: Phil Ewels <[email protected]>
|
Who is going to maintain this? |
|
This looks OK - the only change with flux is we deprecated mini, so it's just flux submit. |
|
Thanks @vsoch! Are you happy / do you understand the logic behind the other changes to the flags? (I haven't read up on them yet so don't). |
|
Also do you mean that flux / flux mini are not two different systems? So we can just make this change and it's valid for all people using flux? We might need some docs updates here as well. |
modules/nextflow/src/main/groovy/nextflow/executor/FluxExecutor.groovy
Outdated
Show resolved
Hide resolved
modules/nextflow/src/main/groovy/nextflow/executor/FluxExecutor.groovy
Outdated
Show resolved
Hide resolved
modules/nextflow/src/main/groovy/nextflow/executor/FluxExecutor.groovy
Outdated
Show resolved
Hide resolved
|
And this title is not correct:
They are exactly the same. |
ewels
changed the title
|
@ewels not sure what to do with this PR |
|
@ewels could you please address my questions/comments so we can move the PR forward? |
|
@vsoch because I just pasted the code diff from an email, I don't know the answers sorry. I'll follow up with him again and try to find out. |
Signed-off-by: Phil Ewels <[email protected]>
Signed-off-by: Phil Ewels <[email protected]>
ewels
changed the title
mini from submit command
Signed-off-by: Phil Ewels <[email protected]>
|
ok after feedback from the original author, this PR gets much simpler - now just a single line PR. I've also removed the Is this good to merge now @vsoch? Thanks for the review, and apologies for the slightly odd nature of this PR. |
|
This now LGTM! 👍 |
Signed-off-by: Phil Ewels <[email protected]>
|
Tests green, there are some unresolved comments |
Co-authored-by: Christopher Hakkaart <[email protected]> Signed-off-by: Phil Ewels <[email protected]>
|
Comments all resolved 👌🏻 |
Porting a set of changes from an email thread for discussion.
Context is that a user was trying to use Nextflow with Flux. The executor is written for flux mini and didn't work for him.
After some back and forth and investigation, he found the following set of changes got the executor to work on their system. This led to the creation of the following docs page for the system users: https://doku.lrz.de/nextflow-on-hpc-systems-test-operation-788693597.html
This was all only in an email thread, so moving here so that it doesn't get lost. I'm curious whether the flux executor support could be tweaked to work with both flux core and flux mini? (maybe with some configuration?). The changes seem pretty minor.
'cc @pditommaso and I think the original implementation was by @vsoch