Story #10293

[Crunch2] Add output_uuid field to container_request

Added by Peter Amstutz over 4 years ago. Updated over 4 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Radhika Chippada
Category:
-
Target version:
Start date:
11/11/2016
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
0.5
Release:
Release relationship:
Auto

Description

Container requests, when finalized, copy the container output to a new collection. However, the uuid of this collection is not recorded on the container request, although the collection has a 'properties' field pointing to the container request. The container request should have an output_uuid field storing the collection that was created for the container request.

For migration add a non-existent zzzz-* collection UUID.

Update arvados-cwl-runner to read new field and change to monitor container requests rather than containers.


Subtasks

Task #10501: Review branch 10293-container-request-output-uuidResolvedRadhika Chippada

Task #10555: Review branch 10293-cwl-cr-outputResolvedPeter Amstutz

Task #10660: Review 10293-cr-log-and-output-in-wbResolvedLucas Di Pentima

Associated revisions

Revision ce924ad9
Added by Radhika Chippada over 4 years ago

refs #10293
Merge branch '10293-container-request-output-uuid'

Revision 0d2bed24
Added by Peter Amstutz over 4 years ago

Merge branch 'origin-10293-cwl-cr-output' closes #10293

Revision 03f81200 (diff)
Added by Peter Amstutz over 4 years ago

Fix bug done.done() not returning output value. refs #10293

Revision beeab71d
Added by Radhika Chippada over 4 years ago

refs #10293
Merge branch '10293-cr-log-and-output-in-wb'

History

#1 Updated by Peter Amstutz over 4 years ago

  • Description updated (diff)

#2 Updated by Peter Amstutz over 4 years ago

  • Subject changed from [Crunch2] Container request output is a collection uuid to [Crunch2] Add output_uuid field to container_request

#3 Updated by Tom Morris over 4 years ago

  • Description updated (diff)
  • Target version set to 2016-11-23 sprint
  • Story points set to 2.0

#4 Updated by Radhika Chippada over 4 years ago

  • Assigned To set to Radhika Chippada

#5 Updated by Radhika Chippada over 4 years ago

  • Status changed from New to In Progress

#6 Updated by Radhika Chippada over 4 years ago

Branch 10293-container-request-output-uuid implements API updates.

Please elaborate on "update arvados-cwl-runner to read new field and change to monitor container requests rather than containers."

Thanks.

#7 Updated by Peter Amstutz over 4 years ago

I had a thought. Should we add a log_uuid column, similar to the output_uuid column?

For cwl-runner:

  • arvcontainer.py ArvadosContainer.run()

Change this to response["uuid"] to record the container request uuid:

            self.arvrunner.processes[response["container_uuid"]] = self

Don't fetch the container, so this block can be removed, as well as subsequent references to the "container" variable.

            container = self.arvrunner.api.containers().get(
                uuid=response["container_uuid"]
            ).execute(num_retries=self.arvrunner.num_retries)

Change this to test the container request state: if response["state"] "Final":

if container["state"] in ("Complete", "Cancelled"):
  • In ArvadosContainer.done()

The "record" variable is going to be a container_request and not a container.

As a result, this needs to make an API call to get the container record. Then it can check if state "Complete" and get exit_code.

Change record["output"] to record["output_uuid"]:

                if record["output"]:
                    outputs = done.done(self, record, "/tmp", self.outdir, "/keep")
  • Similar changes need to happen to arvcontaine.py RunnerContainer
  • In init.py ArvCwlRunner.poll_states()

Change this to self.poll_api.container_requests()

            if self.work_api == "containers":
                table = self.poll_api.containers()
  • In init.py ArvCwlRunner.on_message()

Add "Final" to the list of states:

event["properties"]["new_attributes"]["state"] in ("Complete", "Failed", "Cancelled"):

#8 Updated by Peter Amstutz over 4 years ago

"tom: tetron: yes to log_uuid"

Please add "log_uuid" which is handled the same way as "output_uuid".

#9 Updated by Radhika Chippada over 4 years ago

Branch 10293-container-request-output-uuid addresses the API server related updates @ a078c965

The cwl related updates will be implemented in a separate branch.

#10 Updated by Peter Amstutz over 4 years ago

10293-container-request-output-uuid @ 4bccbaed84c6b398f4cb4dbc7a9bc345e79d6550 LGTM

#11 Updated by Radhika Chippada over 4 years ago

Branch 10293-cwl-cr-output implements the CWL updates and documentation updates at c40265a8

Test run @ https://ci.curoverse.com/job/developer-run-tests/72/

#12 Updated by Peter Amstutz over 4 years ago

I thought we fixed this? (If so, merge master so I can test)

arvados.cwl-runner[12922] ERROR: Got error <HttpError 422 when requesting https://192.168.5.2:8000/arvados/v1/container_requests?alt=json returned "Scheduling parameters must be a Hash, not a ActiveSupport::HashWithIndifferentAccess">

This is wrong, it still needs to be assigning the value of "self":

-            self.arvrunner.processes[response["container_uuid"]] = self
+            self.arvrunner.processes[response["uuid"]] = response["uuid"]

The number of "%s" doesn't match up with the number of parameters:

logger.info("Container request %s (%s) state is %s with container %s %s", self.name, response["uuid"], response["state"])

This needs to check that container["outputs"] is set before calling done_outputs. It also needs to have try/except around done_outputs. The exception handler needs to call output_callback({}, "permanentFail").

            outputs = done.done_outputs(self, container, "/tmp", self.outdir, "/keep")
            self.output_callback(outputs, processStatus)

#13 Updated by Radhika Chippada over 4 years ago

@ e4a5141

This is wrong, it still needs to be assigning the value of "self":

Changed it back to "self".

The number of "%s" doesn't match up with the number of parameters:

Oops. Fixed it

This needs to check that container["outputs"] is set before calling done_outputs.

The code is already doing the check and there is no "outputS" in container?

     if container["output"]:
                outputs = done.done_outputs(self, container, "/tmp", self.outdir, "/keep")

It also needs to have try/except around done_outputs. The exception handler needs to call output_callback({}, "permanentFail").

Added the try/except block around it.

Also, the output_callback was being done "outside" of ' if container["output"] ' check earlier and hence I preserved it.

Thanks.

#14 Updated by Radhika Chippada over 4 years ago

  • Target version changed from 2016-11-23 sprint to 2016-12-14 sprint

#15 Updated by Radhika Chippada over 4 years ago

  • Story points changed from 2.0 to 0.5

#16 Updated by Peter Amstutz over 4 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 50 to 100

Applied in changeset arvados|commit:0d2bed2452f0840612be0b1bc792ffeff576d065.

#17 Updated by Lucas Di Pentima over 4 years ago

9e9190af52e4fc448f623aa2f4dd1fee803d99cd at 10293-cr-log-and-output-in-wb branch LGTM.

Also available in: Atom PDF