From 8dc0adf44a18e48471706ad1cf9c6ef1ec8eef95 Mon Sep 17 00:00:00 2001 From: Paolo Di Tommaso Date: Mon, 14 Sep 2020 22:30:23 +0200 Subject: [PATCH] Dx green with nf-core/rnaseq --- .../executor/BashWrapperBuilder.groovy | 12 +++-- .../aws/batch/AwsBatchFileCopyStrategy.groovy | 2 +- .../nextflow/cloud/aws/util/S3PathTest.groovy | 45 +++++++++++++++++++ .../main/nextflow/extension/FilesEx.groovy | 13 +++--- .../file/FileSystemPathFactory.groovy | 31 ++++++++++++- .../cloud/google/util/GsPathFactory.groovy | 10 ++++- .../google/util/GsPathFactoryTest.groovy | 13 +++--- 7 files changed, 107 insertions(+), 19 deletions(-) create mode 100644 modules/nf-amazon/src/test/nextflow/cloud/aws/util/S3PathTest.groovy diff --git a/modules/nextflow/src/main/groovy/nextflow/executor/BashWrapperBuilder.groovy b/modules/nextflow/src/main/groovy/nextflow/executor/BashWrapperBuilder.groovy index 27bf77f391..d5d79a81f9 100644 --- a/modules/nextflow/src/main/groovy/nextflow/executor/BashWrapperBuilder.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/executor/BashWrapperBuilder.groovy @@ -441,6 +441,10 @@ class BashWrapperBuilder { throw new IllegalArgumentException("Unknown container engine: $engine") } + protected boolean getAllowContainerMounts() { + return true + } + /** * Build a {@link DockerBuilder} object to handle Docker commands * @@ -458,16 +462,18 @@ class BashWrapperBuilder { * initialise the builder */ // do not mount inputs when they are copied in the task work dir -- see #1105 - if( stageInMode != 'copy' ) + if( stageInMode != 'copy' && allowContainerMounts ) builder.addMountForInputs(inputFiles) - builder.addMount(binDir) + if( allowContainerMounts ) + builder.addMount(binDir) if(this.containerMount) builder.addMount(containerMount) // task work dir - builder.setWorkDir(workDir) + if( allowContainerMounts ) + builder.setWorkDir(workDir) // set the name builder.setName('$NXF_BOXID') diff --git a/modules/nf-amazon/src/main/nextflow/cloud/aws/batch/AwsBatchFileCopyStrategy.groovy b/modules/nf-amazon/src/main/nextflow/cloud/aws/batch/AwsBatchFileCopyStrategy.groovy index 1ec158160b..25894ed65f 100644 --- a/modules/nf-amazon/src/main/nextflow/cloud/aws/batch/AwsBatchFileCopyStrategy.groovy +++ b/modules/nf-amazon/src/main/nextflow/cloud/aws/batch/AwsBatchFileCopyStrategy.groovy @@ -59,7 +59,7 @@ class AwsBatchFileCopyStrategy extends SimpleFileCopyStrategy { @Override String getEnvScript(Map environment, boolean container) { if( container ) - throw new IllegalArgumentException("Parameter `wrapHandler` not supported by ${this.class.simpleName}") + throw new IllegalArgumentException("Parameter `container` not supported by ${this.class.simpleName}") final result = new StringBuilder() final copy = environment ? new HashMap(environment) : Collections.emptyMap() diff --git a/modules/nf-amazon/src/test/nextflow/cloud/aws/util/S3PathTest.groovy b/modules/nf-amazon/src/test/nextflow/cloud/aws/util/S3PathTest.groovy new file mode 100644 index 0000000000..c183f7c372 --- /dev/null +++ b/modules/nf-amazon/src/test/nextflow/cloud/aws/util/S3PathTest.groovy @@ -0,0 +1,45 @@ +package nextflow.cloud.aws.util + +import nextflow.file.FileHelper +import spock.lang.Specification +import spock.lang.Unroll + +/** + * + * @author Paolo Di Tommaso + */ +class S3PathTest extends Specification { + + @Unroll + def 'should convert to uri string' () { + + expect: + FileHelper.asPath(PATH).toUriString() == STR + + where: + _ | PATH | STR + _ | 's3://foo' | 's3://foo/' + _ | 's3://foo/bar' | 's3://foo/bar' + _ | 's3://foo/b a r' | 's3://foo/b a r' + _ | 's3://f o o/bar' | 's3://f o o/bar' + _ | 's3://f_o_o/bar' | 's3://f_o_o/bar' + + } + + @Unroll + def 'should convert to string' () { + + expect: + FileHelper.asPath(PATH).toString() == STR + + where: + _ | PATH | STR + _ | 's3://foo' | '/foo/' + _ | 's3://foo/bar' | '/foo/bar' + _ | 's3://foo/b a r' | '/foo/b a r' + _ | 's3://f o o/bar' | '/f o o/bar' + _ | 's3://f_o_o/bar' | '/f_o_o/bar' + + } + +} diff --git a/modules/nf-commons/src/main/nextflow/extension/FilesEx.groovy b/modules/nf-commons/src/main/nextflow/extension/FilesEx.groovy index 233f7ce769..1279ee5ab8 100644 --- a/modules/nf-commons/src/main/nextflow/extension/FilesEx.groovy +++ b/modules/nf-commons/src/main/nextflow/extension/FilesEx.groovy @@ -17,8 +17,7 @@ package nextflow.extension -import org.codehaus.groovy.runtime.InvokerHelper -import static java.nio.file.StandardCopyOption.REPLACE_EXISTING +import static java.nio.file.StandardCopyOption.* import java.nio.ByteBuffer import java.nio.channels.SeekableByteChannel @@ -43,6 +42,7 @@ import groovy.transform.stc.ClosureParams import groovy.transform.stc.FromString import groovy.util.logging.Slf4j import nextflow.file.FileHelper +import nextflow.file.FileSystemPathFactory import nextflow.io.ByteBufferBackedInputStream import nextflow.util.CharsetHelper import nextflow.util.CheckHelper @@ -1492,11 +1492,10 @@ class FilesEx { return path.toString() if( scheme == 's3' ) return "$scheme:/$path".toString() - if( scheme == 'gs' ) { - final bucket = InvokerHelper.invokeMethod(path, 'bucket', InvokerHelper.EMPTY_ARGS) - return "$scheme://$bucket$path".toString() - } - return path.toUri().toString() + // + final result = FileSystemPathFactory.getUriString(path) + // + return result ?: path.toUri().toString() } static String getScheme(Path path) { diff --git a/modules/nf-commons/src/main/nextflow/file/FileSystemPathFactory.groovy b/modules/nf-commons/src/main/nextflow/file/FileSystemPathFactory.groovy index 27fa295957..b52c79e6cb 100644 --- a/modules/nf-commons/src/main/nextflow/file/FileSystemPathFactory.groovy +++ b/modules/nf-commons/src/main/nextflow/file/FileSystemPathFactory.groovy @@ -29,10 +29,29 @@ import groovy.transform.Memoized @CompileStatic abstract class FileSystemPathFactory { + /** + * Converts path uri string to the corresponding {@link Path} object + * + * @param uri + * A fully qualified path uri including the protocol scheme + * @return + * A {@link Path} for the given path or {@code null} if protocol is unknown + */ abstract protected Path parseUri(String uri) + /** + * Converts a {@link Path} object to a fully qualified uri string + * + * @param path + * The {@link Path} object to be converted + * @return + * The uri string corresponding to the specified path or {@code null} + * if the no provider is found + */ + abstract protected String toUriString(Path path) + static Path parse(String uri) { - def factories = factories0() + final factories = factories0() for( int i=0; i factories0() { final result = new ArrayList(10) diff --git a/modules/nf-google/src/main/nextflow/cloud/google/util/GsPathFactory.groovy b/modules/nf-google/src/main/nextflow/cloud/google/util/GsPathFactory.groovy index ebf90f48dd..6c579326ac 100644 --- a/modules/nf-google/src/main/nextflow/cloud/google/util/GsPathFactory.groovy +++ b/modules/nf-google/src/main/nextflow/cloud/google/util/GsPathFactory.groovy @@ -20,12 +20,12 @@ import java.nio.file.Path import com.google.cloud.storage.contrib.nio.CloudStorageConfiguration import com.google.cloud.storage.contrib.nio.CloudStorageFileSystem +import com.google.cloud.storage.contrib.nio.CloudStoragePath import groovy.transform.CompileStatic import nextflow.Global import nextflow.Session import nextflow.cloud.google.lifesciences.GoogleLifeSciencesConfig import nextflow.file.FileSystemPathFactory - /** * Implements FileSystemPathFactory interface for Google storage * @@ -62,4 +62,12 @@ class GsPathFactory extends FileSystemPathFactory { ? CloudStorageFileSystem.forBucket(str, storageConfig).getPath('') : CloudStorageFileSystem.forBucket(str.substring(0,p), storageConfig).getPath(str.substring(p)) ) } + + @Override + protected String toUriString(Path path) { + if( path instanceof CloudStoragePath ) { + return "gs://${path.bucket()}$path".toString() + } + return null + } } diff --git a/modules/nf-google/src/test/nextflow/cloud/google/util/GsPathFactoryTest.groovy b/modules/nf-google/src/test/nextflow/cloud/google/util/GsPathFactoryTest.groovy index dabb5083a8..acaac514bc 100644 --- a/modules/nf-google/src/test/nextflow/cloud/google/util/GsPathFactoryTest.groovy +++ b/modules/nf-google/src/test/nextflow/cloud/google/util/GsPathFactoryTest.groovy @@ -36,14 +36,15 @@ class GsPathFactoryTest extends Specification { expect: factory.parseUri(PATH).toUriString() == PATH + factory.parseUri(PATH).toString() == STR where: - _ | PATH - _ | 'gs://foo' - _ | 'gs://foo/bar' - _ | 'gs://foo/b a r' - _ | 'gs://f o o/bar' - _ | 'gs://f_o_o/bar' + _ | PATH | STR + _ | 'gs://foo' | '' + _ | 'gs://foo/bar' | '/bar' + _ | 'gs://foo/b a r' | '/b a r' + _ | 'gs://f o o/bar' | '/bar' + _ | 'gs://f_o_o/bar' | '/bar' } def 'should use requester pays' () {