-
Notifications
You must be signed in to change notification settings - Fork 360
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
Workflow url in run mode #3988
Workflow url in run mode #3988
Changes from 9 commits
32138fb
099231d
aefa58b
5c3d5c4
06f611f
59cc112
ff2b315
757617f
ff80636
f7007e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
name: invalid_workflow_url_length | ||
testFormat: submitfailure | ||
|
||
files { | ||
workflowUrl: "https://this_url_has_more_than_2000_characters/why_would_someone_have_such_long_urls_one_would_ask/beats_me/now_starts_lorem_ipsum/At_vero_eos_et_accusamus_et_iusto_odio_dignissimos_ducimus_qui_blanditiis_praesentium_voluptatum_deleniti_atque_corrupti_quos_dolores_et_quas_molestias_excepturi_sint_occaecati_cupiditate_non_provident,_similique_sunt_in_culpa_qui_officia_deserunt_mollitia_animi,_id_est_laborum_et_dolorum_fuga._Et_harum_quidem_rerum_facilis_est_et_expedita_distinctio._Nam_libero_tempore,_cum_soluta_nobis_est_eligendi_optio_cumque_nihil_impedit_quo_minus_id_quod_maxime_placeat_facere_possimus,_omnis_voluptas_assumenda_est,_omnis_dolor_repellendus._Temporibus_autem_quibusdam_et_aut_officiis_debitis_aut_rerum_necessitatibus_saepe_eveniet_ut_et_voluptates_repudiandae_sint_et_molestiae_non_recusandae._Itaque_earum_rerum_hic_tenetur_a_sapiente_delectus,_ut_aut_reiciendis_voluptatibus_maiores_alias_consequatur_aut_perferendis_doloribus_asperiores_repellat/Sed_ut_perspiciatis_unde_omnis_iste_natus_error_sit_voluptatem_accusantium_doloremque_laudantium,_totam_rem_aperiam,_eaque_ipsa_quae_ab_illo_inventore_veritatis_et_quasi_architecto_beatae_vitae_dicta_sunt_explicabo._Nemo_enim_ipsam_voluptatem_quia_voluptas_sit_aspernatur_aut_odit_aut_fugit,_sed_quia_consequuntur_magni_dolores_eos_qui_ratione_voluptatem_sequi_nesciunt._Neque_porro_quisquam_est,_qui_dolorem_ipsum_quia_dolor_sit_amet,_consectetur,_adipisci_velit,_sed_quia_non_numquam_eius_modi_tempora_incidunt_ut_labore_et_dolore_magnam_aliquam_quaerat_voluptatem._Ut_enim_ad_minima_veniam,_quis_nostrum_exercitationem_ullam_corporis_suscipit_laboriosam,_nisi_ut_aliquid_ex_ea_commodi_consequatur?_Quis_autem_vel_eum_iure_reprehenderit_qui_in_ea_voluptate_velit_esse_quam_nihil_molestiae_consequatur,_vel_illum_qui_dolorem_eum_fugiat_quo_voluptas_nulla_pariatur?/Lorem_ipsum_dolor_sit_amet,_consectetur_adipiscing_elit,_sed_do_eiusmod_tempor_incididunt_ut_labore_et_dolore_magna_aliqua._Ut_enim_ad_minim_veniam,_quis_nostrud_exercitation_ullamco_laboris_nisi_ut_aliquip_ex_ea_commodo_consequat._Duis_aute_irure_dolor_in_reprehenderit_in_voluptate_velit_esse_cillum_dolore_eu_fugiat_nulla_pariatur._Excepteur_sint_occaecat_cupidatat_non_proident,_sunt_in_culpa_qui_officia_deserunt_mollit_anim_id_est_laborum/hello.wdl/hello.wdl" | ||
} | ||
|
||
submit { | ||
statusCode: 400 | ||
message: """{ | ||
"status": "fail", | ||
"message": "Error(s): Invalid workflow url: url has length 2305, longer than the maximum allowed 2000 characters" | ||
}""" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
<?xml version="1.0" encoding="UTF-8" standalone="no"?> | ||
<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog" | ||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.3.xsd"> | ||
|
||
<changeSet author="sshah" id="change_max_size_for_workflow_url"> | ||
<modifyDataType tableName="WORKFLOW_STORE_ENTRY" columnName="WORKFLOW_URL" newDataType="VARCHAR(2000)"/> | ||
</changeSet> | ||
|
||
</databaseChangeLog> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ package cromwell.webservice | |
import java.net.URL | ||
|
||
import _root_.io.circe.yaml | ||
import akka.http.scaladsl.model.IllegalUriException | ||
import akka.util.ByteString | ||
import cats.data.NonEmptyList | ||
import cats.data.Validated._ | ||
|
@@ -59,6 +58,8 @@ object PartialWorkflowSources { | |
|
||
val allPrefixes = List(WorkflowInputsAuxPrefix) | ||
|
||
val MaxWorkflowUrlLength = 2000 | ||
|
||
def fromSubmitRoute(formData: Map[String, ByteString], | ||
allowNoInputs: Boolean): Try[Seq[WorkflowSourceFilesCollection]] = { | ||
import cats.instances.list._ | ||
|
@@ -206,16 +207,12 @@ object PartialWorkflowSources { | |
} | ||
} | ||
|
||
def validateWorkflowUrl(workflowUrl: Option[String]): ErrorOr[Option[WorkflowUrl]] = { | ||
workflowUrl.traverse(convertStringToUrl) | ||
} | ||
|
||
partialSources match { | ||
case Valid(partialSource) => | ||
(validateInputs(partialSource), | ||
validateOptions(partialSource.workflowOptions), | ||
validateLabels(partialSource.customLabels.getOrElse("{}")), | ||
validateWorkflowUrl(partialSource.workflowUrl)) mapN { | ||
partialSource.workflowUrl.traverse(validateWorkflowUrl)) mapN { | ||
case (wfInputs, wfOptions, workflowLabels, wfUrl) => | ||
wfInputs.map(inputsJson => WorkflowSourceFilesCollection( | ||
workflowSource = partialSource.workflowSource, | ||
|
@@ -239,11 +236,17 @@ object PartialWorkflowSources { | |
JsObject(convertToMap reduce (_ ++ _)) | ||
} | ||
|
||
def convertStringToUrl(workflowUrl: String): ErrorOr[WorkflowUrl] = { | ||
Try(new URL(workflowUrl)) match { | ||
case Success(_) => workflowUrl.validNel | ||
case Failure(e: IllegalUriException) => s"Invalid workflow url: ${e.getMessage}".invalidNel | ||
case Failure(e) => s"Error while validating workflow url: ${e.getMessage}".invalidNel | ||
def validateWorkflowUrl(workflowUrl: String): ErrorOr[WorkflowUrl] = { | ||
def convertStringToUrl(workflowUrl: String): ErrorOr[WorkflowUrl] = { | ||
Try(new URL(workflowUrl)) match { | ||
case Success(_) => workflowUrl.validNel | ||
case Failure(e) => s"Error while validating workflow url: ${e.getMessage}".invalidNel | ||
} | ||
} | ||
|
||
workflowUrl.length match { | ||
case l if l > MaxWorkflowUrlLength => s"Invalid workflow url: url has length $l, longer than the maximum allowed $MaxWorkflowUrlLength characters".invalidNel | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor point but this whole There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It sure can! 👍 |
||
case _ => convertStringToUrl(workflowUrl) | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,10 +11,11 @@ import com.typesafe.config.ConfigFactory | |
import common.exception.MessageAggregation | ||
import common.validation.ErrorOr._ | ||
import cromwell.CommandLineArguments.ValidSubmission | ||
import cromwell.CommandLineArguments.WorkflowSourceOrUrl | ||
import cromwell.CromwellApp._ | ||
import cromwell.api.CromwellClient | ||
import cromwell.api.model.{Label, LabelsJsonFormatter, WorkflowSingleSubmission} | ||
import cromwell.core.path.Path | ||
import cromwell.core.path.{DefaultPathBuilder, Path} | ||
import cromwell.core.{WorkflowSourceFilesCollection, WorkflowSourceFilesWithDependenciesZip, WorkflowSourceFilesWithoutImports} | ||
import cromwell.engine.workflow.SingleWorkflowRunnerActor | ||
import cromwell.engine.workflow.SingleWorkflowRunnerActor.RunWorkflow | ||
|
@@ -175,17 +176,24 @@ object CromwellEntryPoint extends GracefulStopSupport { | |
import spray.json._ | ||
|
||
val validation = args.validateSubmission(EntryPointLogger) map { | ||
case ValidSubmission(w, u, r, i, o, l, z) => | ||
case ValidSubmission(w, u, r, i, o, l, z) =>{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I like to add braces if it make things easier to read. But, here it does not matter as there is just one case! So, will remove it. |
||
val finalWorkflowSourceAndUrl: WorkflowSourceOrUrl = { | ||
if (w.isDefined) WorkflowSourceOrUrl(w,u) // submission has CWL workflow file path and no imports | ||
else if (u.get.startsWith("http")) WorkflowSourceOrUrl(w, u) | ||
else WorkflowSourceOrUrl(Option(DefaultPathBuilder.get(u.get).contentAsString), None) //case where url is a WDL/CWL file | ||
} | ||
|
||
WorkflowSingleSubmission( | ||
workflowSource = w, | ||
workflowUrl = u, | ||
workflowSource = finalWorkflowSourceAndUrl.source, | ||
workflowUrl = finalWorkflowSourceAndUrl.url, | ||
workflowRoot = r, | ||
workflowType = args.workflowType, | ||
workflowTypeVersion = args.workflowTypeVersion, | ||
inputsJson = Option(i), | ||
options = Option(o), | ||
labels = Option(l.parseJson.convertTo[List[Label]]), | ||
zippedImports = z) | ||
} | ||
} | ||
|
||
validOrFailSubmission(validation) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
cwlVersion: v1.0 | ||
class: CommandLineTool | ||
requirements: | ||
- class: InlineJavascriptRequirement | ||
hints: | ||
DockerRequirement: | ||
dockerPull: "debian:stretch-slim" | ||
inputs: [] | ||
baseCommand: [touch, z, y, x, w, c, b, a] | ||
outputs: | ||
letters: | ||
type: string | ||
outputBinding: | ||
glob: '?' | ||
outputEval: | | ||
${ return self.sort(function(a,b) { return a.location > b.location ? 1 : (a.location < b.location ? -1 : 0) }).map(f => f.basename).join(" ") } |
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.
I would say "it can be either a local path or a remote URL pointing to the workflow source file"