Skip to content

Conversation

@achave11-ucsc
Copy link
Collaborator

@achave11-ucsc achave11-ucsc commented Aug 1, 2018

Making a different pr for the tests that contain fixes for the bugs reported and fixed in #48.
Included new test for checking workflow definition dependency.

…les to the workflow need to

be attached to a workflow_attachment. Thereby uploaded to a tem location
for the server. workflow_url should always be assigned to the workflow
descriptor file, as described by wes-schemas.
@achave11-ucsc achave11-ucsc mentioned this pull request Aug 1, 2018
@achave11-ucsc achave11-ucsc requested a review from tetron August 1, 2018 17:11
@achave11-ucsc achave11-ucsc changed the title Tests fixes for #48 Tests fixes for #48, includes #48 changes and fixes for #47 Aug 1, 2018
workflow_url could be any secondary file required by the workflow.
Copy link
Collaborator

@DailyDreaming DailyDreaming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@tetron
Copy link
Member

tetron commented Aug 1, 2018

From chat:

https://github.com/achave11/workflow-service/blob/57f43ff2909b6377560ef07c04b3070aa45508fa/wes_service/arvados_wes.py#L188

This block is obsolete. The earlier definition of "workflow_descriptor" was for it to contain literal text but that was superceded by the multipart upload strategy. The code that saves the literal text from "workflow_descriptor" to a file should be removed.

@achave11-ucsc achave11-ucsc merged commit 2982869 into common-workflow-language:master Aug 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants