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

[BUG] Pipeline.get_status_tree returns no slots for aborted pipeline with no children #47

Open
ppastuszkaocado opened this issue Jul 27, 2015 · 3 comments

Comments

@ppastuszkaocado
Copy link

We have a following pipeline:

class SomePipeline(pipeline.Pipeline):
    output_names = ['status']

    def run(self, execution_plan):
        status, error = // do external call

        self.fill(self.outputs.status, json.dumps({
            'status': str(status),
            'error': error
        }))

        //...

        if status == ERROR:
            raise Abort(error)

        if status == DONE:
            yield pipeline.common.Return(str(status))

Now, let's assume we're running this single pipeline and the Abort exception is thrown and the pipeline is aborted, but the slot was filled before that happened. Unfortunately, when I try to ask for Pipeline.get_status_tree() I get something like:

{
  "pipelines": {
    "eac5905d74e042aeae9ff30f60d5e09d": {
      "outputs": {
        "default": "ahpkZXZ-ZGV2LWF0bS1ldS1kYXRhbWFuYWdlcnJwCxITX0FFX1BpcGVsaW5lX1JlY29yZCIgZWFjNTkwNWQ3NGUwNDJhZWFlOWZmMzBmNjBkNWUwOWQMCxIRX0FFX1BpcGVsaW5lX1Nsb3QiIGMyNmQ3Nzg0MzNhMTRmYmQ4ZTBkYmI5Njc3NWIyZmZjDA",
        "status": "ahpkZXZ-ZGV2LWF0bS1ldS1kYXRhbWFuYWdlcnJwCxITX0FFX1BpcGVsaW5lX1JlY29yZCIgZWFjNTkwNWQ3NGUwNDJhZWFlOWZmMzBmNjBkNWUwOWQMCxIRX0FFX1BpcGVsaW5lX1Nsb3QiIGI1NTFkYjExOWFiMjQxMTRhZmRlMGQ2NGZiZWMxYmIyDA"
      },
       // other elements here
      "children": [],
      "status": "aborted"
    }
  },
  "slots": {},
  "rootPipelineId": "eac5905d74e042aeae9ff30f60d5e09d"
}

Please, observe, that "slots" is an empty object, although we got appropriate keys in the "outputs" section.

@tkaitchuck
Copy link
Contributor

This is indeed a strange outcome, but this is also strange code. I am not sure what the right thing here is.
Why do you have code filling the outputs above the status check? What do you expect the caller to see in that case?

@ppastuszkaocado
Copy link
Author

@tkaitchuck - The whole scenario is more complex then you see here. We autogenerate pipeline DAG in the code (we call them workflows), so in general there is more than one pipeline (the single-pipeline workflow example, I've posted above, is a special case). When one of the pipeline in a DAG fails, it's actually a critical situation for the whole workflow and all pipelines in the DAG should be aborted (that's why we raise Abort there). Later on, when we display the whole DAG status in our UI (using output generated by get_status_tree()), we would like to show user particular pipeline that has failed - we cannot depend on built-in pipeline status as all pipelines in the DAG have 'aborted' state in case of Abort exception being raised. That's why all our pipelines has "status" slot.

It looks like problem is in this line: https://github.com/GoogleCloudPlatform/appengine-pipelines/blob/master/python/src/pipeline/pipeline.py#L3178
The slots are added to the status tree only for the "child" pipelines, i.e. slot is not added for root pipeline. Is there a reason for it?

I'd like to open a pull request for this issue, but I'm having problems with running unit tests. Is there some README how to run them?

@tkaitchuck
Copy link
Contributor

I added a shell script to make it easier.
d6078c7

Run that with the argument 'test' and set the environment variable to point to your SDK.

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

2 participants