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

Implementation of store RDF export of the workflow in CWL Prov RO-Bundle #1709

Draft
wants to merge 55 commits into
base: main
Choose a base branch
from

Conversation

jjkoehorst
Copy link
Contributor

@jjkoehorst jjkoehorst commented Aug 16, 2022

Keeping as draft while we test proposed enhancements to CWLProv

Added file size to prov RO-Bundle

Creates a workflow.ttl file in the default folder while running.
Then closes the file and after the provenance folder is created it is copied to the provenance/workflow/ folder.

temp directory creation and removal added

…dle.

Added file size to prov RO-Bundle

Creates a workflow.ttl file in the default folder while running.
Then closes the file and after the provenance folder is created it is copied to the provenance/workflow/ folder.

Update cwltool/main.py

Co-authored-by: Michael R. Crusoe <[email protected]>

temp directory creation and removal added
@jjkoehorst jjkoehorst requested a review from mr-c August 16, 2022 14:29
@codecov
Copy link

codecov bot commented Aug 16, 2022

Codecov Report

Merging #1709 (420dd1c) into main (5a645df) will decrease coverage by 11.95%.
The diff coverage is 11.11%.

@@             Coverage Diff             @@
##             main    #1709       +/-   ##
===========================================
- Coverage   83.91%   71.96%   -11.95%     
===========================================
  Files          46       46               
  Lines        8150     8269      +119     
  Branches     2169     2167        -2     
===========================================
- Hits         6839     5951      -888     
- Misses        842     1832      +990     
- Partials      469      486       +17     
Files Changed Coverage Δ
cwltool/job.py 75.83% <ø> (-6.10%) ⬇️
cwltool/loghandler.py 91.30% <0.00%> (-8.70%) ⬇️
cwltool/utils.py 83.39% <ø> (-1.14%) ⬇️
cwltool/workflow.py 84.16% <ø> (-7.73%) ⬇️
cwltool/cwlprov/ro.py 23.65% <7.46%> (-50.80%) ⬇️
cwltool/cwlprov/provenance_profile.py 10.00% <8.69%> (-64.09%) ⬇️
cwltool/main.py 63.47% <30.76%> (-10.94%) ⬇️
cwltool/argparser.py 87.89% <100.00%> (-7.39%) ⬇️

... and 21 files with indirect coverage changes

jjkoehorst and others added 6 commits August 16, 2022 16:44
Fix needed for the type for the activity function in adding the workflow step
looking into accessing the arguments parser from the checksum_only option so we can add a --no-data like option to cwltool
…hould be used and if the SHA1 string can be a URI (faster indexing in triple stores)
@jjkoehorst jjkoehorst requested a review from kinow August 17, 2022 06:18
Copy link
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

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

GH Actions is showing some linter errors. I think running make format should fix the code locally, then you can review if it modified only the files you modified, and push it again (sometimes it modifies more than what I changed, and then CI keeps failing; just ignore if make format changes more than what you expected).

@jjkoehorst
Copy link
Contributor Author

GH Actions is showing some linter errors. I think running make format should fix the code locally, then you can review if it modified only the files you modified, and push it again (sometimes it modifies more than what I changed, and then CI keeps failing; just ignore if make format changes more than what you expected).

Thanks many jobs now fail as well due to:

E           bagit.BagValidationError: Bag validation failed: data/26/260807aefd9032a48a0d88d4ff7110a5bbfaea3d sha1 validation failed: expected="260807aefd9032a48a0d88d4ff7110a5bbfaea3d" found="da39a3ee5e6b4b0d3255bfef95601890afd80709"; data/ff/ff1f8b01ee9e74ec5f9840aa07ea6bc2895a235e sha1 validation failed: expected="ff1f8b01ee9e74ec5f9840aa07ea6bc2895a235e" found="da39a3ee5e6b4b0d3255bfef95601890afd80709"

As I am removing the provenance /data files and will turn it into an option later. This way we can use input files again that are ±100gb in size and prevent them from being copied to the data folder

@lgtm-com
Copy link

lgtm-com bot commented Aug 17, 2022

This pull request introduces 6 alerts when merging 21ecba9 into f3a35f3 - view on LGTM.com

new alerts:

  • 2 for Unused import
  • 2 for Unreachable code
  • 2 for Module-level cyclic import

@lgtm-com
Copy link

lgtm-com bot commented Aug 17, 2022

This pull request introduces 6 alerts when merging 879d5ce into f3a35f3 - view on LGTM.com

new alerts:

  • 2 for Unused import
  • 2 for Unreachable code
  • 2 for Module-level cyclic import

@lgtm-com
Copy link

lgtm-com bot commented Aug 18, 2022

This pull request introduces 3 alerts when merging 211348a into f3a35f3 - view on LGTM.com

new alerts:

  • 2 for Module-level cyclic import
  • 1 for Unused import

@kinow kinow mentioned this pull request Aug 18, 2022
@lgtm-com
Copy link

lgtm-com bot commented Aug 24, 2022

This pull request introduces 3 alerts when merging bd61e43 into 0e2ced5 - view on LGTM.com

new alerts:

  • 2 for Module-level cyclic import
  • 1 for Unused import

cwltool/main.py Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Aug 25, 2022

This pull request introduces 2 alerts when merging ad90be6 into 0e2ced5 - view on LGTM.com

new alerts:

  • 2 for Module-level cyclic import

@lgtm-com
Copy link

lgtm-com bot commented Sep 7, 2022

This pull request introduces 1 alert when merging d3048af into 87f3b01 - view on LGTM.com

new alerts:

  • 1 for Unused import

@@ -48,7 +48,7 @@ def cwltool(tmp_path: Path, *args: Any) -> Path:
def cwltool_no_data(tmp_path: Path, *args: Any) -> Path:
prov_folder = tmp_path / "provenance"
prov_folder.mkdir()
new_args = ["--no-data", "--provenance", str(prov_folder)]
new_args = ["--enable-ext", "--no-data", "--provenance", str(prov_folder)]
Copy link
Member

Choose a reason for hiding this comment

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

--enable-ext shouldn't be required when using loadListing with CWL v1.1+

@lgtm-com
Copy link

lgtm-com bot commented Sep 8, 2022

This pull request introduces 1 alert when merging ac532d4 into 1d23218 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Sep 8, 2022

This pull request introduces 1 alert when merging fb5a65a into 1d23218 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 12, 2022

This pull request introduces 1 alert when merging d01a0df into 6f0e1d9 - view on LGTM.com

new alerts:

  • 1 for Use of the return value of a procedure

@lgtm-com
Copy link

lgtm-com bot commented Oct 12, 2022

This pull request introduces 1 alert when merging 315e78f into 6f0e1d9 - view on LGTM.com

new alerts:

  • 1 for Unused import

# Conflicts:
#	.gitignore
#	cwltool/cwlprov/provenance_profile.py
#	cwltool/cwlprov/ro.py
#	cwltool/executors.py
#	cwltool/main.py
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