Skip to content
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

WX-1461 Remove womtool upgrade command #7382

Merged
merged 19 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## 87 Release Notes

### `upgrade` command removed from Womtool

Womtool previously supported a `womtool upgrade` command for upgrading draft-2 WDLs to 1.0. With WDL 1.1 soon to become the latest supported version, this functionality is retiring.

### Replacement of `gsutil` with `gcloud storage`

In this release, all **localization** functionality on the GCP backend migrates to use the more modern and performant `gcloud storage`. With sufficiently powerful worker VMs, Cromwell can now localize at up to 1200 MB/s [0][1][2].
Expand Down
53 changes: 0 additions & 53 deletions centaur/src/it/scala/centaur/AbstractCentaurTestCaseSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -76,59 +76,6 @@ abstract class AbstractCentaurTestCaseSpec(cromwellBackends: List[String],
runOrDont(testCase, tags, isIgnored, retries, runTestAndDeleteZippedImports())
}

def executeWdlUpgradeTest(testCase: CentaurTestCase): Unit =
executeStandardTest(wdlUpgradeTestWdl(testCase))

private def wdlUpgradeTestWdl(testCase: CentaurTestCase): CentaurTestCase = {
import better.files.File
import womtool.WomtoolMain

// The suffix matters because WomGraphMaker.getBundle() uses it to choose the language factory
val rootWorkflowFile = File.newTemporaryFile(suffix = ".wdl").append(testCase.workflow.data.workflowContent.get)
val workingDir = File.newTemporaryDirectory()
val upgradedImportsDir = File.newTemporaryDirectory()
val rootWorkflowFilepath = workingDir / rootWorkflowFile.name

// Un-upgraded imports go into the working directory
testCase.workflow.data.zippedImports match {
case Some(importsZip: File) =>
importsZip.unzipTo(workingDir)
case None => ()
}

// Upgrade the imports and copy to main working dir (precludes transitive imports; no recursion yet)
workingDir.list.toList.map { file: File =>
val upgradedWdl = WomtoolMain.upgrade(file.pathAsString).stdout.get
upgradedImportsDir.createChild(file.name).append(upgradedWdl)
}

// Copy to working directory after we operate on the imports that are in it
rootWorkflowFile.copyTo(rootWorkflowFilepath)

val upgradeResult = WomtoolMain.upgrade(rootWorkflowFilepath.pathAsString)

upgradeResult.stderr match {
case Some(stderr) => println(stderr)
case _ => ()
}

val newCase = testCase.copy(
workflow = testCase.workflow.copy(
testName = testCase.workflow.testName + " (draft-2 to 1.0 upgrade)",
data = testCase.workflow.data.copy(
workflowContent = Option(upgradeResult.stdout.get), // this '.get' catches an error if upgrade fails
zippedImports = Option(upgradedImportsDir.zip())
)
)
)(cromwellTracker) // An empty zip appears to be completely harmless, so no special handling

rootWorkflowFile.delete(swallowIOExceptions = true)
upgradedImportsDir.delete(swallowIOExceptions = true)
workingDir.delete(swallowIOExceptions = true)

newCase
}

private def runOrDont(testCase: CentaurTestCase,
tags: List[Tag],
ignore: Boolean,
Expand Down
2 changes: 0 additions & 2 deletions centaur/src/it/scala/centaur/CentaurTestSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ object CentaurTestSuite extends StrictLogging {
logger.info(s"Cromwell under test configured with backends ${cromwellBackends.mkString(", ")}")
logger.info(s"Unless overridden by workflow options file, tests use default backend: $defaultBackend")

def isWdlUpgradeTest(testCase: CentaurTestCase): Boolean = testCase.containsTag("wdl_upgrade")

def isEngineUpgradeTest(testCase: CentaurTestCase): Boolean = testCase.containsTag("engine_upgrade")

def isPapiUpgradeTest(testCase: CentaurTestCase): Boolean = testCase.containsTag("papi_upgrade")
Expand Down
15 changes: 0 additions & 15 deletions centaur/src/it/scala/centaur/WdlUpgradeTestCaseSpec.scala

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
name: composedenginefunctions
testFormat: workflowsuccess
tags: ["wdl_upgrade"]

files {
workflow: composedenginefunctions/composedenginefunctions.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: ifs_in_scatters
testFormat: workflowsuccess
tags: [ conditionals, "wdl_upgrade" ]
tags: [ conditionals ]

files {
workflow: conditionals_tests/ifs_in_scatters/ifs_in_scatters.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: nested_lookups
testFormat: workflowsuccess
tags: [ conditionals, "wdl_upgrade" ]
tags: [ conditionals ]

files {
workflow: conditionals_tests/nested_lookups/nested_lookups.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: scatters_in_ifs
testFormat: workflowsuccess
tags: [ conditionals, "wdl_upgrade" ]
tags: [ conditionals ]

files {
workflow: conditionals_tests/scatters_in_ifs/scatters_in_ifs.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: simple_if
testFormat: workflowsuccess
tags: [ conditionals, "wdl_upgrade" ]
tags: [ conditionals ]

files {
workflow: conditionals_tests/simple_if/simple_if.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: simple_if_workflow_outputs
testFormat: workflowsuccess
tags: [ conditionals, "wdl_upgrade" ]
tags: [ conditionals ]

files {
workflow: conditionals_tests/simple_if_workflow_outputs/simple_if_workflow_outputs.wdl
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

name: declarations
testFormat: workflowsuccess
tags: ["wdl_upgrade"]

files {
workflow: declarations/declarations.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
name: declarations_as_nodes
testFormat: workflowsuccess
tags: ["wdl_upgrade"]

files {
workflow: declarations_as_nodes/declarations_as_nodes.wdl
Expand Down
1 change: 0 additions & 1 deletion centaur/src/main/resources/standardTestCases/hello.test
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
name: hello
testFormat: workflowsuccess
tags: ["wdl_upgrade"]

files {
workflow: hello/hello.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
name: object_access
testFormat: workflowsuccess
tags: ["wdl_upgrade"]

files {
workflow: object_access/object_access.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
name: scatterchain
testFormat: workflowsuccess
tags: ["wdl_upgrade"]

files {
workflow: scatter_chain/scatter_chain.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
name: scattergather
testFormat: workflowsuccess
tags: ["wdl_upgrade"]

files {
workflow: scattergather/scattergather.wdl
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: short_circuit
testFormat: workflowsuccess

backends: [Local]
tags: [localdockertest, "wdl_upgrade"]
tags: [localdockertest]


files {
Expand Down
1 change: 0 additions & 1 deletion centaur/src/main/resources/standardTestCases/square.test
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
name: square
testFormat: workflowsuccess
tags: [ "wdl_upgrade" ]

files {
workflow: square/square.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
name: string_interpolation
testFormat: workflowsuccess
tags: ["wdl_upgrade"]

files {
workflow: string_interpolation/string_interpolation.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: sub_workflow_hello_world
testFormat: workflowsuccess
tags: [subworkflow, "wdl_upgrade"]
tags: [subworkflow]

files {
workflow: sub_workflow_hello_world/sub_workflow_hello_world.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: sub_workflow_var_refs
testFormat: workflowsuccess
tags: [subworkflow, "wdl_upgrade"]
tags: [subworkflow]

files {
workflow: sub_workflow_var_refs/sub_workflow_var_refs.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: valid_labels
testFormat: workflowsuccess
tags: [ labels, "wdl_upgrade" ]
tags: [ labels ]

files {
workflow: hello/hello.wdl
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
name: write_lines
testFormat: workflowsuccess
tags: ["wdl_upgrade"]

files {
workflow: write_lines/write_lines.wdl
Expand All @@ -13,4 +12,3 @@ metadata {
"calls.write_lines.f2a.executionStatus": Done
"outputs.write_lines.a2f_second.x": "a\nb\nc\nd"
}

5 changes: 1 addition & 4 deletions docs/developers/Centaur.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ our tests into groups depending on which configuration files they require. Below
| PAPI Upgrade | `PapiUpgradeTestCaseSpec` | `papiUpgradeTestCases` |
| PAPI Upgrade<br>New Workflows | `CentaurTestSuite` | `papiUpgradeNewWorkflowsTestCases` |
| Azure Blob | `CentaurTestSuite ` | `azureBlobTestCases` |
| WDL Upgrade | `WdlUpgradeTestCaseSpec` | `standardTestCases`**** |
| (other) | `CentaurTestSuite` | `standardTestCases` |

<small>
Expand All @@ -143,9 +142,7 @@ or `papi_v2beta_centaur_application.conf`
\*\* Cromwell Config overrides
([47 link](https://github.com/broadinstitute/cromwell/blob/47/src/ci/bin/test.inc.sh#L213-L221))
\*\*\* Test Directory overrides
([47 link](https://github.com/broadinstitute/cromwell/blob/47/src/ci/bin/test.inc.sh#L440-L449))
\*\*\*\* Test Directory only tests tagged with `wdl_upgrade`
([47 link](https://github.com/broadinstitute/cromwell/blob/47/centaur/src/main/resources/standardTestCases/write_lines.test#L3))
([47 link](https://github.com/broadinstitute/cromwell/blob/47/src/ci/bin/test.inc.sh#L440-L449))
</small>

- Engine Upgrade: Retrieves the [Cromwell Version](https://github.com/broadinstitute/cromwell/blob/47/project/Version.scala#L8) then retrieves the previous jar/docker-image from DockerHub. Centaur starts with the prior version, then restarts with the compiled source code.
Expand Down
1 change: 0 additions & 1 deletion src/ci/bin/test.inc.sh
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ cromwell::private::create_build_variables() {
backend_type="${CROMWELL_BUILD_TYPE}"
backend_type="${backend_type#centaurEngineUpgrade}"
backend_type="${backend_type#centaurPapiUpgrade}"
backend_type="${backend_type#centaurWdlUpgrade}"
backend_type="${backend_type#centaurHoricromtal}"
backend_type="${backend_type#centaur}"
backend_type="$(echo "${backend_type}" | sed 's/\([A-Z]\)/_\1/g' | tr '[:upper:]' '[:lower:]' | cut -c 2-)"
Expand Down
19 changes: 0 additions & 19 deletions src/ci/bin/testCentaurWdlUpgradeLocal.sh

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,9 @@ final case class CallElement(callableReference: String,
body: Option[CallBodyElement],
override val sourceLocation: Option[SourceFileLocation]
) extends LanguageElement
with WorkflowGraphElement
with WorkflowGraphElement {
override def toString: String =
s"""Call "$callableReference${alias.map(alias => s" as $alias").getOrElse("")}${afters
.map(after => s" after $after")
.mkString}""""
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ final case class IntermediateValueDeclarationElement(typeElement: TypeElement,
name: String,
expression: ExpressionElement
) extends WorkflowGraphElement
with TaskSectionElement
with TaskSectionElement {
override def toString: String = s"""Declaration "$name" ($typeElement)"""
}

object IntermediateValueDeclarationElement {
def fromContent(content: DeclarationContent): IntermediateValueDeclarationElement =
Expand All @@ -24,7 +26,9 @@ object IntermediateValueDeclarationElement {
*/
final case class OutputDeclarationElement(typeElement: TypeElement, name: String, expression: ExpressionElement)
extends LanguageElement
with WorkflowGraphElement
with WorkflowGraphElement {
override def toString: String = s"""Output "$name" ($typeElement)"""
}

object OutputDeclarationElement {
def fromContent(content: DeclarationContent): OutputDeclarationElement =
Expand All @@ -36,7 +40,9 @@ object OutputDeclarationElement {
*/
final case class InputDeclarationElement(typeElement: TypeElement, name: String, expression: Option[ExpressionElement])
extends LanguageElement
with WorkflowGraphElement
with WorkflowGraphElement {
override def toString: String = s"""Input "$name" ($typeElement)"""
}

object DeclarationElement {
/* Custom unapply so that elsewhere we can do things like this, and otherwise treat all declarations the same:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
package wdl.model.draft3.elements

final case class IfElement(conditionExpression: ExpressionElement, graphElements: Seq[WorkflowGraphElement])
extends WorkflowGraphElement
extends WorkflowGraphElement {
override def toString: String = s"""Condition "$conditionExpression""""
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,6 @@ final case class ScatterElement(scatterName: String,
// Shorthand to only include certain members for hashing purposes
// https://stackoverflow.com/a/31915429/818054
override def hashCode(): Int = (scatterExpression, scatterVariableName, graphElements).##

override def toString: String = s"""Scatter "$scatterName" ($scatterExpression)"""
}
Loading
Loading