Allow for more than one container image

[cjllanwarne]

No description provided.

[denis-yuen]

Question for when I'm back, is this backwards compatible? i.e. are the swagger code clients or the Python (bravado?) client that @jaeddy uses resilient to this change? If so, I'm inclined to approve this.

[denis-yuen]

One second thought, both values are optional anyway.
I think this is fine.

[ypriverol]

@denis-yuen in our specification we are using three metrics:

full_tag: quay.io/seqware/seqware_full/1.1
size: Size of the container in bytes
updated: Last time the container was updated.
type: 'Docker' or 'Singularity' or 'Conda'

An example here: http://api.biocontainers.pro/api/v2/tools/sc3-scripts/versions

[denis-yuen]

@ypriverol @susheel
those sound reasonable to me as long as they are optional, do you feel making a new PR with those values?

[denis-yuen]

bumping @ypriverol @susheel

[geoffjentry]

NB getting the size of a docker container isn't particularly straightforward

[ghost]

Thanks for the use case @geoffjentry (via GA4Gh chat).

My opinion is that such workflow specific data is best left out of the specification. I am biased towards lean specifications to encourage adoption and reduce maintenance.

To your point re: multiple API calls for the docker images, once the workflow has been sucked in, the recipient is free to parse the workflow and - if that's how it works - pull all the docker images in at once.

I did not understand the part about extra API calls because in my mental model the client is going to pull the workflow document anyway, so it will get this information one way or the other.

Thanks!

[denis-yuen]

NB getting the size of a docker container isn't particularly straightforward

For example in our case, we don't actually have this info, hence why I'd prefer it to be optional if anything.

[rishidev]

Driver Project representatives were emailed on 2019-03-08 for NCI GDC, ENA/EVA/EGA, Genomics England, CanDIG, TopMed with request for comments and votes by registered 2019-03-18. HCA input provided via Brian, All Of Us from David Glazer, ELIXIR Cloud from Susheel.

[rdeborj]

Allowing for more than one container image will be useful for CanDIG as we plan on leveraging both docker and singularity containers.

[geoffjentry]

@rdeborj OOI and largely off topic - are you talking about using native singularity containers or singularity to run docker containers? It sounds like the former.

Not asking as pushback in any way, just that I haven't come across too many people in the bioinfx world looking to do the former and so when I do I want to make sure I understand their use case more fully.

I'm also happy to take this offline (jgentry at broadinstitute dot org)

[denis-yuen]

Allowing for more than one container image will be useful for CanDIG as we plan on leveraging both docker and singularity containers.

Interesting, I had maybe assumed this as something like "my workflow uses n containers rather than 1 container, you can pre-load/cache them all if you want" as opposed to "my workflow can use container X but it may also use container Y as an alternative"

@ypriverol could you confirm which use-case you had in mind?

[ghost]

@rdeborj how will the docker images be mapped for equivalency? A workflow will tend to have multiple images. I still think this is an app property rather than anything else.

So, perhaps your runner is able to use multiple images because of the root perms issue (singularity) or because you've found it better for resiliency to rely on multiple docker repositories but then this is more cleanly described - if we consider CWL - in an extension tag that says, for each tool,

candig:alternate_images:
  - image1
  - image2
  - image3

[rdeborj]

@geoffjentry I was thinking native singularity usage. Our group out in Vancouver uses singularity on their HPC and the request would satisfy their use. Docker has been a hard no in their environment.

@denis-yuen good point, I just assumed most workflows would use more than 1 container and that the questions at hand was based on different supported container types.

@kaushik-work can you clarify what you mean by "mapped for equivalency"?

[geoffjentry]

@rdeborj That's why I was asking. A lot of folks use singularity to run docker containers in those situations, or they would if they realized that that works. OTOH if they're already using native Singularity containers, c'est la vie

[ghost]

@rdeborj sorry for not being clear:

A workflow will contain multiple tools. Each tool will have a set of equivalent docker images. Presumably the tool description will itself have to carry this information.

If we duplicate this information in the metadata returned by the API then we should not have just a flat list of images, but possibly mirror the entire data in the workflow/tools, since the images have particular meanings. So imageB is the singularity equivalent of imageA in tool12 of this workflow.

If as @geoffjentry suggested this is just to prefetch all images as a pile and sort them out later once the workflow document is received, this organization does not matter, but I'm still not clear on why we need this, since we get this from the workflow document anyway :(.

[ypriverol]

I'm happy with the current proposal @denis-yuen We can make optional the tag, size and lat_update

Can't review own work

[denis-yuen]

@kaushik-work

If as @geoffjentry suggested this is just to prefetch all images as a pile and sort them out later once the workflow document is received, this organization does not matter, but I'm still not clear on why we need this, since we get this from the workflow document anyway :(.

I would agree that it is essentially redundant if the user (code) understands the workflow language which is why I don't want to make it required. However, as with #59 I think part of the desire here is that the user might not understand the language. For example, in that case, the workflow testbed needed different behaviour to send CWL draft-2 workflows to some WES endpoints and CWL 1.0 workflows to others (or it might be WDL, I may be misremembering).

@geoffjentry @rdeborj
If the equivalency feature is worth pursuing/baking into the standard then please open a new issue

@ypriverol
I've modified the PR to include your optional fields, I can't really review anymore so pinging people for reviews

[geoffjentry]

@denis-yuen I've said a few times that the suggestions @cjllanwarne made were in the vein of trying to align with a similar-but-not-the same sort of project and thus if they didn't seem to make sense for TRS to feel free to 86 them. So no worries on this side.

[ghost]

@denis-yuen thanks for the clarification! I think it will work in some cases but I've seen several WDL workflows where the docker image is a variable - i.e. injected at run time, so in the end the client can not rely on this list at least for WDL (for CWL a simple parse of the yaml will yield this information easily).