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

YAN-424 - CWL output to stdout #22

Merged
merged 9 commits into from
Sep 24, 2020
Merged

YAN-424 - CWL output to stdout #22

merged 9 commits into from
Sep 24, 2020

Conversation

james-strauss-uwa
Copy link
Collaborator

Make the "dlg cwl" command output a zip file to stdout when no output file is specified by the user.

The main change is in tool_commands.py, where the cwl command now uses _open_o() with opts.output to send output to stdout.

Other changes are:

  • modified input parameters to create_workflow() and create_command_line_tool(). Previously, these methods wrote the intermediate workflow and step files to disk, before combining the files into a zip file for output. Now the contents of the workflow and steps are held in strings, used to create io.BytesIO objects, and the zip file is built in memory.
  • added a common.s2b() method, since the io.BytesIO() method requires "byte-like" input in Python 3. I'd be interested in feedback on this approach.

@coveralls
Copy link

coveralls commented Sep 23, 2020

Coverage Status

Coverage decreased (-0.09%) to 75.418% when pulling a33c237 on yan-424 into 4ce7018 on master.

Copy link
Contributor

@rtobar rtobar left a comment

Choose a reason for hiding this comment

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

Looks great! My only suggestion is around passing the final file object as a parameter to create_workflow instead of accumulating in memory first; that way the writing is done directly to the final "target".

daliuge-translator/dlg/dropmake/cwl.py Outdated Show resolved Hide resolved
daliuge-translator/dlg/dropmake/cwl.py Outdated Show resolved Hide resolved
daliuge-translator/dlg/dropmake/cwl.py Outdated Show resolved Hide resolved
daliuge-translator/dlg/dropmake/web/lg_web.py Outdated Show resolved Hide resolved
daliuge-translator/dlg/translator/tool_commands.py Outdated Show resolved Hide resolved
daliuge-translator/test/dropmake/test_pg_gen.py Outdated Show resolved Hide resolved
daliuge-common/dlg/common/__init__.py Outdated Show resolved Hide resolved
daliuge-common/dlg/common/__init__.py Outdated Show resolved Hide resolved
daliuge-translator/dlg/dropmake/cwl.py Outdated Show resolved Hide resolved
Copy link
Contributor

@rtobar rtobar left a comment

Choose a reason for hiding this comment

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

Looks even better now, thanks for the great work :)

@james-strauss-uwa james-strauss-uwa merged commit 8f146c1 into master Sep 24, 2020
@rtobar rtobar deleted the yan-424 branch July 7, 2021 07:23
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.

None yet

3 participants