New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove square brackets from job name in LSF executor #4799
Conversation
✅ Deploy Preview for nextflow-docs-staging canceled.
|
Signed-off-by: Brian Fulton-Howard <fultonh1@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting this PR. I left a minor comment, also it would be nice to add a unit test for the stripping of the brackets.
An example here
modules/nextflow/src/main/groovy/nextflow/executor/LsfExecutor.groovy
Outdated
Show resolved
Hide resolved
That link didn't work for me. How about adding the following to LsfExecutorTest.groovy: // Adapted from PbsExecutorTest.groovy
@Unroll
def 'should return valid job name given #name'() {
given:
def executor = [:] as LsfExecutor
def task = Mock(TaskRun)
task.getName() >> name
expect:
executor.getJobNameFor(task) == expected
executor.getJobNameFor(task).size() <= 4094
where:
name | expected
'hello' | 'nf-hello'
'12 45' | 'nf-12_45'
'hello[123]' | 'nf-hello123'
'a'.repeat(4095) | 'nf-'.concat("a".repeat(4091))
} My apologies if there's anything wrong with this. I am not really a Java programmer and I am pretty new to the workings of Nextflow. edit: I just ran Gradle and the test seems to have worked:
|
Signed-off-by: Brian Fulton-Howard <brian.fulton-howard@mssm.edu> Adapted from PbsExecutorTest.groovy
modules/nextflow/src/test/groovy/nextflow/executor/LsfExecutorTest.groovy
Outdated
Show resolved
Hide resolved
…Test.groovy Co-authored-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> Signed-off-by: Brian Fulton-Howard <fultonh1@gmail.com>
Signed-off-by: Brian Fulton-Howard <brian.fulton-howard@mssm.edu>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this contribution
… [ci fast] Signed-off-by: Brian Fulton-Howard <fultonh1@gmail.com> Signed-off-by: Brian Fulton-Howard <brian.fulton-howard@mssm.edu> Co-authored-by: Brian Fulton-Howard <brian.fulton-howard@mssm.edu> Co-authored-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> Signed-off-by: Niklas Schandry <niklas@bio.lmu.de>
… [ci fast] Signed-off-by: Brian Fulton-Howard <fultonh1@gmail.com> Signed-off-by: Brian Fulton-Howard <brian.fulton-howard@mssm.edu> Co-authored-by: Brian Fulton-Howard <brian.fulton-howard@mssm.edu> Co-authored-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> Signed-off-by: Niklas Schandry <niklas@bio.lmu.de>
When there is a task name with square brackets, it fails on my LSF cluster:
...
The brackets are responsible for that error. Therefore, I have modified
sanitizeJobName()
from the PBS executor to fix this.