From d22cff6aa80f41da38b1d54be6c94369656370a7 Mon Sep 17 00:00:00 2001 From: Dima Ryazanov Date: Mon, 16 Oct 2023 12:46:49 -0600 Subject: [PATCH] Throw exceptions instead of using status codes (#150) --- .../main/nextflow/quilt/QuiltProduct.groovy | 22 +++--------- .../nextflow/quilt/jep/QuiltPackage.groovy | 11 ++---- .../nextflow/quilt/QuiltProductTest.groovy | 36 +++++++++++++++---- .../quilt/jep/QuiltPackageTest.groovy | 30 +++++++++++----- 4 files changed, 57 insertions(+), 42 deletions(-) diff --git a/plugins/nf-quilt/src/main/nextflow/quilt/QuiltProduct.groovy b/plugins/nf-quilt/src/main/nextflow/quilt/QuiltProduct.groovy index ee04edc6..c887e66c 100644 --- a/plugins/nf-quilt/src/main/nextflow/quilt/QuiltProduct.groovy +++ b/plugins/nf-quilt/src/main/nextflow/quilt/QuiltProduct.groovy @@ -117,7 +117,6 @@ ${nextflow} private final Session session private String msg private Map meta - final Integer pubStatus QuiltProduct(QuiltPath path, Session session) { this.path = path @@ -125,35 +124,22 @@ ${nextflow} this.msg = pkg.toString() this.meta = [pkg: msg, time_start: now()] this.session = session - this.pubStatus = -1 if (session.isSuccess() || pkg.is_force()) { - try { - this.pubStatus = publish() - } - catch (Exception e) { - log.error("publish failed: ${e.getMessage()}", pkg.meta) - } + publish() } else { log.info("not publishing: ${pkg} [unsuccessful session]") } } - int publish() { + void publish() { log.debug("publish($msg)") meta = setupMeta() String text = setupReadme() log.debug("setupReadme: $text") List quilt_summarize = setupSummarize() log.debug("setupSummarize: $quilt_summarize") - int rc = pkg.push(msg, meta) - log.info("$rc: pushed package[$pkg] $msg") - if (rc > 0) { - print("ERROR[package push failed]: $pkg\n") - } else { - print("SUCCESS: $pkg\n") - } - - return rc + pkg.push(msg, meta) + print("SUCCESS: $pkg\n") } boolean shouldSkip(key) { diff --git a/plugins/nf-quilt/src/main/nextflow/quilt/jep/QuiltPackage.groovy b/plugins/nf-quilt/src/main/nextflow/quilt/jep/QuiltPackage.groovy index 7d0129aa..78726f06 100644 --- a/plugins/nf-quilt/src/main/nextflow/quilt/jep/QuiltPackage.groovy +++ b/plugins/nf-quilt/src/main/nextflow/quilt/jep/QuiltPackage.groovy @@ -221,7 +221,7 @@ class QuiltPackage { }) } // https://docs.quiltdata.com/v/version-5.0.x/examples/gitlike#install-a-package - int push(String msg = 'update', Map meta = [:]) { + void push(String msg = 'update', Map meta = [:]) { S3PhysicalKey registryPath = new S3PhysicalKey(bucket, '', null) Registry registry = new Registry(registryPath) Namespace namespace = registry.getNamespace(packageName) @@ -245,14 +245,7 @@ class QuiltPackage { Manifest m = builder.build() - try { - m.push(namespace, "nf-quilt:${today()}-${msg}", parsed.workflowName) - } catch (WorkflowException e) { - return 1 - } catch (IOException e) { - return 1 - } - return 0 + m.push(namespace, "nf-quilt:${today()}-${msg}", parsed.workflowName) } @Override diff --git a/plugins/nf-quilt/src/test/nextflow/quilt/QuiltProductTest.groovy b/plugins/nf-quilt/src/test/nextflow/quilt/QuiltProductTest.groovy index d3e94904..c839aa97 100644 --- a/plugins/nf-quilt/src/test/nextflow/quilt/QuiltProductTest.groovy +++ b/plugins/nf-quilt/src/test/nextflow/quilt/QuiltProductTest.groovy @@ -141,15 +141,37 @@ class QuiltProductTest extends QuiltSpecification { Map bad_meta = meta + ['Type': 'Workflow'] Map skip_meta = ['metadata': 'SKIP'] + when: + makeWriteProduct() // no metadata + then: + thrown(com.quiltdata.quiltcore.workflows.WorkflowException) + expect: - makeWriteProduct().pubStatus == 1 // no metadata - makeWriteProduct(meta).pubStatus == 0 // valid metadata - makeWriteProduct().pubStatus == 1 // invalid default metadata - makeWriteProduct(bad_meta).pubStatus == 1 // invalid explicit metadata - makeWriteProduct(skip_meta).pubStatus == 1 // no default metadata + makeWriteProduct(meta) // valid metadata + + when: + makeWriteProduct() // invalid default metadata + then: + thrown(com.quiltdata.quiltcore.workflows.WorkflowException) + + when: + makeWriteProduct(bad_meta) // invalid explicit metadata + then: + thrown(com.quiltdata.quiltcore.workflows.WorkflowException) + + when: + makeWriteProduct(skip_meta) // no default metadata + then: + thrown(com.quiltdata.quiltcore.workflows.WorkflowException) + // NOTE: push does NOT update local registry + expect: makeWriteProduct().pkg.install() // try to merge existing metadata - makeWriteProduct(skip_meta).pubStatus == 1 // still fails on implicit metadata + + when: + makeWriteProduct(skip_meta) // still fails on implicit metadata + then: + thrown(com.quiltdata.quiltcore.workflows.WorkflowException) } void writeFile(root, filename) { @@ -188,7 +210,7 @@ class QuiltProductTest extends QuiltSpecification { QuiltProduct product = new QuiltProduct(path, session) expect: - product.publish() == 0 + product.publish() sumPkg.install() Files.exists(Paths.get(sumPkg.packageDest().toString(), QuiltProduct.SUMMARY_FILE)) } diff --git a/plugins/nf-quilt/src/test/nextflow/quilt/jep/QuiltPackageTest.groovy b/plugins/nf-quilt/src/test/nextflow/quilt/jep/QuiltPackageTest.groovy index c3ca540a..1bf6bc20 100644 --- a/plugins/nf-quilt/src/test/nextflow/quilt/jep/QuiltPackageTest.groovy +++ b/plugins/nf-quilt/src/test/nextflow/quilt/jep/QuiltPackageTest.groovy @@ -154,7 +154,10 @@ class QuiltPackageTest extends QuiltSpecification { Files.writeString(outPath, "Time: ${timestamp}") expect: Files.exists(outPath) - opkg.push() != 0 + when: + opkg.push() + then: + thrown(java.io.IOException) } @IgnoreIf({ env.WRITE_BUCKET == 'quilt-example' || env.WRITE_BUCKET == null }) @@ -176,7 +179,7 @@ class QuiltPackageTest extends QuiltSpecification { Files.writeString(outPath, "Time: ${timestamp}") expect: Files.exists(outPath) - opkg.push() == 0 + opkg.push() opkg.reset() !Files.exists(outPath) opkg.install() // returns Path @@ -190,7 +193,7 @@ class QuiltPackageTest extends QuiltSpecification { QuiltPackage opkg = writeablePackage('observer') Map meta = ['key': "val=\'(key)\'"] expect: - opkg.push('msg', meta) == 0 + opkg.push('msg', meta) } @IgnoreIf({ env.WRITE_BUCKET == 'quilt-example' || env.WRITE_BUCKET == null }) @@ -198,8 +201,10 @@ class QuiltPackageTest extends QuiltSpecification { given: String pkgName = "workflow-bad-${timestamp}" QuiltPackage bad_wf = writeablePackage(pkgName, 'missing-workflow') - expect: - bad_wf.push('missing-workflow first time', [:]) == 1 + when: + bad_wf.push('missing-workflow first time', [:]) + then: + thrown(com.quiltdata.quiltcore.workflows.WorkflowException) } @IgnoreIf({ env.WRITE_BUCKET == 'quilt-example' || env.WRITE_BUCKET == null }) @@ -213,10 +218,19 @@ class QuiltPackageTest extends QuiltSpecification { ] Map bad_meta = meta + ['Type': 'Workflow'] QuiltPackage good_wf = writeablePackage('workflow-good', 'my-workflow') + + when: + good_wf.push('empty meta', [:]) + then: + thrown(com.quiltdata.quiltcore.workflows.WorkflowException) + + when: + good_wf.push('bad_meta', bad_meta) + then: + thrown(com.quiltdata.quiltcore.workflows.WorkflowException) + expect: - good_wf.push('empty meta', [:]) == 1 - good_wf.push('bad_meta', bad_meta) == 1 - good_wf.push('my-workflow', meta) == 0 + good_wf.push('my-workflow', meta) } }