search

quintilesims / layer0

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

MAINT: Make l0 deploy list only return active task definitions #480

Closed jparsons04 closed 6 years ago

jparsons04 commented 6 years ago

What does this pull request do? The methods called in the AWS SDK that pull back Task Definitions should only be retrieving Task Definitions that are active. Without filtering Task Definitions by status, inactive Task Definitions can still be retrieved. For Layer0 instances that have been up for a long time, inactive Task Definitions can accumulate and has been known to slow the operation l0 deploy list down considerably.

How should this be tested? Running l0 deploy list operations (both with and without the --all flag)

Checklist ~- [ ] Unit tests~ ~- [ ] Smoke tests (if applicable)~ ~- [ ] System tests (if applicable)~ ~- [ ] Documentation (if applicable)~

jparsons04 commented 6 years ago

This was mainly cherry picked by looking at what I did in this commit: https://github.com/quintilesims/layer0/commit/bd06b90c4578d7d00f676db3e4377cc20716bbf0

diemonster commented 6 years ago

I'm fine with merging this, but in order to look at potential downsides, I'm struggling to think of a time when a Layer0 user would ever want/need an inactive taskDefinition. Possibly rollbacks, but even then the workflow of just making a new deploy has become the norm versus rolling back to an older version. Thoughts @zpatrick ?

zpatrick commented 6 years ago

I don't think you can even target an inactive task definition for rollbacks, so I say merge as-is.