Skip to content

--cachedir needs to take stdout field into account when calculating the hash #408

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

Closed
mr-c opened this issue Jun 2, 2017 · 4 comments · Fixed by #532
Closed

--cachedir needs to take stdout field into account when calculating the hash #408

mr-c opened this issue Jun 2, 2017 · 4 comments · Fixed by #532

Comments

@mr-c
Copy link
Member

mr-c commented Jun 2, 2017

No description provided.

mr-c added a commit that referenced this issue Jul 7, 2017
0f28896 Merge pull request #408 from kapilkd13/master
18fc8d6 Merge branch 'master' into master
d71b2f7 Merge pull request #467 from kapilkd13/inlineJS-conformance
1f5f0de Update conformance_test_v1.0.yaml
8abc3f7 Merge pull request #472 from common-workflow-language/no-draft-links
7134321 remove links to pre-v1.0 from homepage
2a88775 Merge pull request #471 from common-workflow-language/changelong-tweak
0e0cb4b clarify that these change descriptions are specific to each of the standards
da56971 added conformance test for InlineJavascriptRequirement
485be33 Merge pull request #465 from common-workflow-language/video_link
d874786 Add preview & link to intro video
397769d Merge pull request #463 from common-workflow-language/hmenager-patch-1
cab882c version links
ca21bdf moar
84bd265 Update cli-description-languages-comparison.md
0cd5307 Merge pull request #464 from common-workflow-language/bd2k
1cd58d3 Update cli-description-languages-comparison.md
4834980 Add BD2K as a participating organization
80e52b5 Update cli-description-languages-comparison.md
4f1fcd5 Update cli-description-languages-comparison.md
145bd0b Update cli-description-languages-comparison.md
c9ed818 Update cli-description-languages-comparison.md
1da94c2 Update cli-description-languages-comparison.md
1ff1819 Update cli-description-languages-comparison.md
24d782b Update cli-description-languages-comparison.md
e793622 Create cli-description-languages-comparison.md
2625c13 adding conformance test: Testing warning instead of error when default path is not present
52d9fcb changes to enable nolinkcheck in case of default field, related to cwltool issue #2

git-subtree-dir: cwltool/schemas
git-subtree-split: 0f28896
tetron pushed a commit that referenced this issue Jul 23, 2017
changes to enable nolinkcheck in case of default field, related to cwltool issue#2
@kapilkd13
Copy link
Contributor

kapilkd13 commented Aug 12, 2017

Hi @mr-c, here does stdout field means whatever is written to stdout should be used in calculating cache hash. We are calculating cache key here https://github.com/common-workflow-language/cwltool/blob/master/cwltool/draft2tool.py#L298 I mean to ask, how to obtain stdout field inside this function.
Also I personally feel that instead of calculating keydict of values that go inside cachekey calculation in Job function, we can pass it as a argument, starting from main function and propagating to all intermediate function adding value to it whenever necessary and finally resolving inside job function. This can allow us to avoid parameter references that may come inside our prepared cmdline https://github.com/common-workflow-language/cwltool/blob/master/cwltool/draft2tool.py#L274
Is this possible?

@mr-c
Copy link
Member Author

mr-c commented Aug 13, 2017

Here are the ingredients for this issue

  1. type: stdout without and accompanying stdout: my_stdout_name results in a randomly named file that stdout is redirected to. This isn't very reproducible! Suggested fix: make the generation of this name stable as long as the tool nor its inputs change
  2. As you point out, the cache key calculation doesn't take the name of the stdout field into effect. This also need to be fixed

@mr-c
Copy link
Member Author

mr-c commented Aug 14, 2017

@mr-c
Copy link
Member Author

mr-c commented Aug 14, 2017

for 2. Change https://github.com/common-workflow-language/cwltool/blob/master/cwltool/draft2tool.py#L274 to be keydict = {u"cmdline": cmdline, u"stdout", ... }

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 a pull request may close this issue.

2 participants