-
Notifications
You must be signed in to change notification settings - Fork 30
[WIP] chained docker tasks integration test #326
base: master
Are you sure you want to change the base?
Conversation
ab25298
to
d5a17f2
Compare
d5a17f2
to
e245076
Compare
4993b78
to
b612dd2
Compare
7255f0a
to
54b3740
Compare
@app.task(base=DockerTask, bind=True) | ||
def docker_run_chained(task, results, container_args=None, *args, **kwargs): | ||
container_args = container_args or [] | ||
kwargs['container_args'] = [_maybe_transform_chain_result(a, results) for a in container_args] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little confused. You shouldn't need to transform these results in the actual task. The transformation happens when the task class (girder_worker.task.Task
) calls __call__
here
Ideally we'd have a ResultTransform associated with the first docker task that returns something like a GirderFileId
transform. That should get transformed as apart of the above code block before being passed to the second docker_run task. Is there something i'm missing that is preventing that approach?
|
||
class ChainedResultItem(ChainedResultTransform): | ||
def transform(self, results): | ||
item_id = super(ChainedResultItem, self).transform(results) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh i see... do you have multiple results being returned from the first docker task? What are they?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how this actually works, but my understanding of girder_result_hooks was that the return value from a docker_run task was always going to be a list, and I thought it was a list whose values are the resolved result hooks. So in the right-hand task in a chain, we need to be able to select which index into that list is the result we want to use.
I have no idea whether this will actually work. If you have an architecture you think is better, that would be great, but you'd need to write it out in code for me to understand it probably. The code inside test_docker_run_multistep_workflow
in this PR is where we can show how this looks from an API perspective.
No description provided.