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.

Sign up for GitHub

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

Jump to bottom

Symlink pasthrough on WDL is broken again #5031

Closed
adamnovak opened this issue Jul 22, 2024 · 1 comment · Fixed by #5028
Closed

Symlink pasthrough on WDL is broken again #5031

adamnovak opened this issue Jul 22, 2024 · 1 comment · Fixed by #5028

Comments

@adamnovak
Copy link
Member

adamnovak commented Jul 22, 2024 •
edited by unito-bot
Loading

I think as of #4994, and somewhere in the diff between b3a016c and fb3b630, we added a regression and #4850 came back and WDL tasks can no longer output symlinks to their inputs. Test 72 our conformance tests, which tests this, started failing again, but we didn't notice in CI because the test was never marked passing.

We should fix the symlink passthrough again.

We should also maybe set up CI so that if we have a WDL conformance test marked as failing, it's an error if it doesn't fail, to ensure we keep the list up to date when we make fixes.

┆Issue is synchronized with this Jira Story
┆Issue Number: TOIL-1620
@adamnovak adamnovak changed the title Symlink pasthrough on WDL si broken again Symlink pasthrough on WDL is broken again Jul 22, 2024
adamnovak added a commit to adamnovak/toil that referenced this issue Jul 22, 2024
@adamnovak
Put symlink passthrough test on skip list until DataBiosphere#5031 is…
3a33aa2 
… fixed
@stxue1
Copy link
Contributor

stxue1 commented Jul 24, 2024

My guess to why this broke is due to some weird trickery in #4994 while dealing with the sentinel value.

#5028 overhauls the way files are virtualized in toil-wdl-runner, possibly resulting in the sentinel value being obsolete (I still have to figure out what merging #4994 from master into #5028 did behaviorally), so the fix will probably be (replacing the sentinel value? except for task boundaries?) by effectively reverting to ae49ee5 while implementing a new solution to #4988 (likely before the virtualize_files call and outputs, which is where the optional coerced files will likely be used)

@stxue1 stxue1 mentioned this issue Jul 24, 2024
Merged
19 tasks
DailyDreaming pushed a commit that referenced this issue Jul 25, 2024
@adamnovak @github-actions
Fix WDL glob order (#5027)
aab8d2d 
* Expand WDL globs on an actual command line

This should fix #5009.

* Upgrade conformance tests

This lets us maintain one list of unsupported conformance test numbers,
and load the integration tests differently.

It also now brings in a conformance test for glob order.

* Use WDL conformance tests that have a sensible input_override test

* Skip the tests Toil fails on 1.1 and upodate time estimates

* Put 1.0 failing tests on skip list and remove now-passing symlink test

* Put symlink passthrough test on skip list until #5031 is fixed

* Require sphinx-autoapi new enough to have fixed the duplicate object description regression

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@adamnovak adamnovak mentioned this issue Aug 14, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Move file virtualization in toil-wdl-runner to task boundaries DataBiosphere/toil
2 participants
@adamnovak @stxue1