Skip to content
This repository was archived by the owner on May 6, 2020. It is now read-only.

Conversation

Cryptophobia
Copy link
Contributor

We noticed that the api was giving great exceptions when you make a request that fails, but we would like to store those detailed exceptions up to the release api object for version of the release on which they occurred. Similar to how failed=True is available on the release api object, there should be a field called exception containing the exception msg from the deis-controller when a release or build fails.

Let's say we make a /scale post request to POST http://deis.192.168.99.100.nip.io/v2/apps/testing-application/scale with the following payload:

{"cmd": 10}

With this PR, when a release fails this is the example of how the release api response will look:

GET http://deis.192.168.99.100.nip.io/v2/apps/testing-application/releases

{
    "count": 37,
    "next": null,
    "previous": null,
    "results": [
        {
            "uuid": "2f104d63-a023-466d-9023-fb77aad75556",
            "app": "testing-application",
            "owner": "admin",
            "created": "2017-08-28T15:28:01Z",
            "updated": "2017-08-28T15:32:07Z",
            "version": 37,
            "summary": "admin deployed a config that failed",
            "failed": true,
            "exception": "error: (app::deploy): (app::deploy): There was a problem while deploying v36 of testing-application-cmd. Additional information:\nNo nodes are available that match all of the following predicates:: Insufficient cpu (1).",
            "config": "c0d45750-1c5d-4c39-a72e-ae3cf9990d6c",
            "build": "3dca0365-e51b-4d8c-988e-2487cc9a179a"
        },
        {
            "uuid": "a265ac6d-980b-457f-af66-e4d081cd9637",
            "app": "testing-application",
            "owner": "admin",
            "created": "2017-08-28T15:23:57Z",
            "updated": "2017-08-28T15:23:57Z",
            "version": 36,
            "summary": "admin changed lifecycle_post_start cmd",
            "failed": false,
            "exception": null,
            "config": "47561aca-8017-4952-aa6b-5fa0a415a398",
            "build": "3dca0365-e51b-4d8c-988e-2487cc9a179a"
        }
...

With this small change we get a detailed message in the api telling us why a certain release failed that is more detailed than the summary message. As a benefit it is stored on the release API object rather than just given as a response to our API calls:

"exception": "error: (app::deploy): (app::deploy): There was a problem while deploying v36 of testing-application-cmd. Additional information:\nNo nodes are available that match all of the following predicates:: Insufficient cpu (1)."

@deis-admin
Copy link

Thanks for the contribution! Please ensure your commits follow our style guide. This code will be tested once a Deis maintainer reviews it.

@mboersma mboersma added this to the v2.18 milestone Aug 28, 2017
@mboersma
Copy link
Member

Jenkins, add to whitelist.

@Cryptophobia
Copy link
Contributor Author

@mboersma : Fixed the tests. Now I am getting this from Jenkins:

ApplyLayer exit status 1 stdout: stderr: write /usr/lib/python3.5/config-3.5m-x86_64-linux-gnu/libpython3.5m-pic.a: no space left on device

@mboersma
Copy link
Member

mboersma commented Sep 7, 2017

This did finally pass--I worked around the error by adding temporary commands to clean the docker graph up.

This is a pretty ambitious change and honestly we haven't had enough time to review it properly. I'd like to leave it out of the v2.18.0 release but keep the PR open if that's amenable to you @Cryptophobia, so maybe this can be merged into a new community fork of controller when that gets rolling. Let me know what you think.

@mboersma mboersma removed this from the v2.18 milestone Sep 7, 2017
@Cryptophobia
Copy link
Contributor Author

@mboersma : sounds good. we can add it to the fork but we would still appreciate a review.

@Cryptophobia
Copy link
Contributor Author

Cryptophobia commented Sep 21, 2017

This PR has been recreated for the deisthree fork here: teamhephy/controller#1

Should we keep this open or close it to track the progress downstream?

@bacongobbler
Copy link
Member

We should close this since we're not intending on merging this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants