-
Notifications
You must be signed in to change notification settings - Fork 18.7k
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
Expose the machine readable state of a container when listing containers #18966
Conversation
5018648
to
cb61d93
Compare
Change LGTM. We'll need to document this new field in the API reference guide. |
Makes sense to me! LGTM, thanks :-) |
Maybe update the diagram of the state machine too, to match the names of the states? |
ee9e5a0
to
62316f0
Compare
I've updated the documentation for the |
Thanks @mariusGundersen! can you also;
|
OK, so the valid states are: |
73ae55c
to
526a22e
Compare
@thaJeztah, I have updated the documentation, and I have added the ability to filter using the state key. I've also changed the diagram to use the states exposed through the api. If this looks OK I will squash the commits |
Thanks @mariusGundersen if we want to deprecate the "status" filter, that will also affect the command line, and we need to add a deprecation notice, so these docs need to be updated as well if we deprecate this;
I'm not sure yet if we want this, given that filtering by I'll move this PR back to "design review" so that we can discuss the deprecation |
I'll break this pull-request apart into two pull requests: one that adds state to the |
sgtm,that will probably make merging easier, and moves the discussion to another PR. thanks! |
thanks! docs LGTM |
ping @mariusGundersen We need to make a change to docker/engine-api. I'm making a PR to get it quick. See docker/engine-api#45. But for the time beeing, moving back to code-review 😅 |
@mariusGundersen unfortunately, it looks like this just didn't make the cut for the 1.10 release, so the docs changes need to be updated to be in the v1.23 API version 😢 |
I love deadlines, especially the whooshing sound they make as they fly by... |
33c9b26
to
78d0bd7
Compare
Looks like we're still waiting on that engine-api update? |
the vendored version in master should have this field already. |
Ah yep, it's there now, and a duplicate field now. @mariusGundersen Can you remove your changes from vendor since this is now in? |
78d0bd7
to
b66e67b
Compare
Updated documentation to reflect the new State property in the inspect remote api Updated API changes for 1.23 Signed-off-by: Marius Gundersen <me@mariusgundersen.net>
b66e67b
to
2ed72a5
Compare
I've force pushed a fix. I kept the order of the variables from engine-api ( |
LGTM moving again back to doc review since this was previously there |
LGTM ping @vdemeester @MHBauer |
LGTM 🐹 |
Expose the machine readable state of a container when listing containers
PoC implementation of #18962 C