Skip to content

Commit 923ca2f

Browse files
committed
feat(nf-google): make the retry behaviour more consistent, and centralized CLI transport detection
Signed-off-by: Tomiles <116039+tomiles@users.noreply.github.com>
1 parent 7594f30 commit 923ca2f

3 files changed

Lines changed: 29 additions & 4 deletions

File tree

plugins/nf-google/src/main/nextflow/cloud/google/batch/GoogleBatchFileCopyStrategy.groovy

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class GoogleBatchFileCopyStrategy extends SimpleFileCopyStrategy {
4848
* CLI stage-in when a CLI transport is selected and {@code stageInMode=copy}.
4949
*/
5050
private boolean useCliStageInCopy() {
51-
return batchConfig.stageInCopyTransport in [BatchConfig.COPY_TRANSPORT_GCLOUD, BatchConfig.COPY_TRANSPORT_GSUTIL] && 'copy' == stageinMode
51+
return BatchConfig.isCliCopyTransport(batchConfig.stageInCopyTransport) && 'copy' == stageinMode
5252
}
5353

5454
private String effectiveStageOutMode() {
@@ -58,7 +58,7 @@ class GoogleBatchFileCopyStrategy extends SimpleFileCopyStrategy {
5858
private boolean shouldUseCliStageOutCopy() {
5959
if( effectiveStageOutMode() != 'copy' )
6060
return false
61-
return batchConfig.stageOutCopyTransport in [BatchConfig.COPY_TRANSPORT_GCLOUD, BatchConfig.COPY_TRANSPORT_GSUTIL]
61+
return BatchConfig.isCliCopyTransport(batchConfig.stageOutCopyTransport)
6262
}
6363

6464
private boolean needsGoogleBatchBashLib() {
@@ -90,7 +90,9 @@ class GoogleBatchFileCopyStrategy extends SimpleFileCopyStrategy {
9090
return super.stageInputFile(path, targetName)
9191
final gsUri = gsUriForCliStageIn(path)
9292
if( gsUri != null ) {
93-
return "downloads+=(\"nxf_cp_retry nxf_gs_download '${gsUri}' ${Escape.path(targetName)}\")"
93+
return batchConfig.maxTransferAttempts > 1
94+
? "downloads+=(\"nxf_cp_retry nxf_gs_download '${gsUri}' ${Escape.path(targetName)}\")"
95+
: "downloads+=(\"nxf_gs_download '${gsUri}' ${Escape.path(targetName)}\")"
9496
}
9597
return super.stageInputFile(path, targetName)
9698
}

plugins/nf-google/src/main/nextflow/cloud/google/batch/client/BatchConfig.groovy

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ class BatchConfig implements ConfigScope {
6262
static final String COPY_TRANSPORT_GSUTIL = 'gsutil'
6363

6464
static final private List<String> VALID_COPY_TRANSPORTS = List.of(COPY_TRANSPORT_POSIX, COPY_TRANSPORT_GCLOUD, COPY_TRANSPORT_GSUTIL)
65+
static final private List<String> CLI_COPY_TRANSPORTS = List.of(COPY_TRANSPORT_GCLOUD, COPY_TRANSPORT_GSUTIL)
6566

6667
static final private int DEFAULT_MAX_SPOT_ATTEMPTS = 0
6768

@@ -253,7 +254,11 @@ class BatchConfig implements ConfigScope {
253254
* Load {@link nextflow.cloud.google.batch.GoogleBatchFileCopyStrategy} when any transport requests CLI object-storage copy.
254255
*/
255256
boolean usesGoogleBatchStaging() {
256-
return [stageInCopyTransport, stageOutCopyTransport].any { it in [COPY_TRANSPORT_GCLOUD, COPY_TRANSPORT_GSUTIL] }
257+
return [stageInCopyTransport, stageOutCopyTransport].any { isCliCopyTransport(it) }
258+
}
259+
260+
static boolean isCliCopyTransport(String transport) {
261+
return transport in CLI_COPY_TRANSPORTS
257262
}
258263

259264
BatchRetryConfig getRetryConfig() { retry }

plugins/nf-google/src/test/nextflow/cloud/google/batch/GoogleBatchFileCopyStrategyTest.groovy

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,24 @@ class GoogleBatchFileCopyStrategyTest extends Specification {
4343
def path = Paths.get('/mnt/disks/b/data/in.txt')
4444
def copy = new GoogleBatchFileCopyStrategy(bean, batch)
4545

46+
expect:
47+
copy.stageInputFile(path, 'in.txt') == 'downloads+=("nxf_gs_download \'gs://b/data/in.txt\' in.txt")'
48+
}
49+
50+
def 'should build gs download staging line with retry when maxTransferAttempts > 1' () {
51+
given:
52+
def batch = new BatchConfig([
53+
stageInCopyTransport: 'gcloud',
54+
maxTransferAttempts: 3
55+
])
56+
def bean = Mock(TaskBean) {
57+
getWorkDir() >> Paths.get('/mnt/disks/w/x')
58+
getTargetDir() >> Paths.get('/mnt/disks/w/x')
59+
getStageInMode() >> 'copy'
60+
}
61+
def path = Paths.get('/mnt/disks/b/data/in.txt')
62+
def copy = new GoogleBatchFileCopyStrategy(bean, batch)
63+
4664
expect:
4765
copy.stageInputFile(path, 'in.txt') == 'downloads+=("nxf_cp_retry nxf_gs_download \'gs://b/data/in.txt\' in.txt")'
4866
}

0 commit comments

Comments
 (0)