quintilesims / layer0

Build, Manage, and Deploy Your Applications
Apache License 2.0
44 stars 20 forks source link

Deploy Provider: List only Active Task Definitions #376

Closed jparsons04 closed 7 years ago

jparsons04 commented 7 years ago

What does this pull request do? Deploy List() will now explicitly only return active Task Definitions and Task Definition Families.

How should this be tested? Manually test Deploy methods with Swagger Unit tests

Checklist

jparsons04 commented 7 years ago

Something I'm concerned about with this implementation is the fact that we are allowing a client to list all inactive Task Definitions and Task Definition Families, but List() currently will only populate Deploy Summaries that have a matching Deploy in the tags db.

In my specific case, I have about 11k inactive Task Definitions (this is normal per AWS's note on this doc page), so when I do an all=true query for List, the process for me can take upwards of 30 seconds, but then I get back a single Deploy that I created through Swagger.

jparsons04 commented 7 years ago

In this case, no, because we're not including the new query parameter, so the swagger controller doesn't change.

                 "get": {
                     Summary: "List all Deploys",
                     Tags:    []string{"Deploy"},
                     Responses: map[string]swagger.Response{
                         "200": {
                             Description: "An array of deploys",
                             Schema:      swagger.NewObjectSliceSchema("Deploy"), 
                         },
                     },
                 },
jparsons04 commented 7 years ago

Likewise, the Deploy controller doesn't change because it's making the same call to DeployProvider.List() as it always has.