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

Undefined behavior for string to file coercion when file does not exist #667

Open
stxue1 opened this issue Jun 21, 2024 · 2 comments
Open

Comments

@stxue1
Copy link
Collaborator

stxue1 commented Jun 21, 2024

It's unclear if a string being coerced to a file should error or not depending on if the file exists and that the file is not outputted in the WDL.

For example:

version 1.1
workflow test_map {
  File f = "path/to/file"

  output {
  }
}

It's unclear whether this workflow should error on File f if the file does not exist.

MiniWDL seems to not error on these cases, but only errors if the file is to be outputted, for example:

version 1.1

workflow test_map {
  File f = "path/to/file"

  output {
    File out = f
  }
}

A lot of the SPEC tests also depend on this behavior, for example:

wdl/SPEC.md

Lines 1190 to 1208 in 664adc3

Example: test_map.wdl
```wdl
version 1.2
workflow test_map {
Map[Int, Int] int_to_int = {1: 10, 2: 11}
Map[String, Int] string_to_int = { "a": 1, "b": 2 }
Map[File, Array[Int]] file_to_ints = {
"/path/to/file1": [0, 1, 2],
"/path/to/file2": [9, 8, 7]
}
output {
Int ten = int_to_int[1] # evaluates to 10
Int b = string_to_int["b"] # evaluates to 2
Array[Int] ints = file_to_ints["/path/to/file1"] # evaluates to [0, 1, 2]
}
}

In particular, this section about type coercion talks about string to file coercion, but doesn't mention whether the file must exist at the coercion step.

Should execution engines ensure that files must exist at coercion or only at a workflow/task output?

@patmagee
Copy link
Member

As of now there is no requirement in the spec for a file declaration to actually exist. You can declare any string as a file, it will simply fail when the engine tries to localize it or read it.

This behaviour is consistent with other programming languages which allow files to be declared without enforcing their existence

@jdidion
Copy link
Collaborator

jdidion commented Jun 26, 2024

Agree w @patmagee. The question is whether we want to make this explicit in the spec. I think we should.

jdidion added a commit that referenced this issue Jun 26, 2024
jdidion added a commit that referenced this issue Jun 26, 2024
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

No branches or pull requests

3 participants