8000 Add completion for stats. by jessfraz · Pull Request #10357 · moby/moby · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add completion for stats. #10357

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 28, 2015
Merged

Add completion for stats. #10357

merged 1 commit into from
Jan 28, 2015

Conversation

jessfraz
Copy link
Contributor

Docker-DCO-1.1-Signed-off-by: Jessica Frazelle jess@docker.com (github: jfrazelle)

@jessfraz jessfraz force-pushed the bash-completion-stats branch from 8737fef to 0884ddb Compare January 26, 2015 19:00
tianon
tianon reviewed Jan 26, 2015
View reviewed changes
contrib/completion/bash/docker
_docker_stats() {
local counter=$(__docker_pos_first_nonflag)
if [ $cword -eq $counter ]; then
__docker_containers_running
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't docker stats take any number of containers as arguments? 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated just some monday morning copy pasta

@jessfraz jessfraz force-pushed the bash-completion-stats branch from 0884ddb to b3dfe1a Compare January 26, 2015 19:16
@jessfraz
Add completion for stats.
b3dfe1a 
Docker-DCO-1.1-Signed-off-by: Jessica Frazelle <jess@docker.com> (github: jfrazelle)
@tianon
Copy link
Member
tianon commented Jan 26, 2015

LGTM, but has pointed out some bizarre behavior I'm seeing (ie, docker stats non_running_container hangs, and I can't see any reason for that client code to be hanging)

@tianon
Copy link
Member
tianon commented Jan 26, 2015

(as opposed to docker stats not_a_container, which comes back immediately with an error)

@jessfraz
Copy link
Contributor Author

@crosbymichael this is because it is waiting for the container to start right?

@jessfraz
Copy link
Contributor Author

I think it is intentional

@tianon
Copy link
Member
tianon commented Jan 26, 2015

So the stats wait for all the containers to start before they display? What if a container stops while I'm viewing stats on it? My dashboard just freezes up? 😦

@icecrime
Copy link
Contributor

I'm pretty sure the ability to call docker stats on a stopped container is intended, and yes metrics will start to broadcast when the container starts. I just tested on multiple containers, and indeed this is very weird but stats only start showing up when every container is running...

@jessfraz
Copy link
Contributor Author

Regardless that shouldn't be a blocker for this :P

@jessfraz
Copy link
Contributor Author

This would be a nice to have for 1.5 ;)

@icecrime icecrime added this to the 1.5.0 milestone Jan 28, 2015
@icecrime
Copy link
Contributor

Agreed: LGTM.

icecrime pushed a commit that referenced this pull request Jan 28, 2015
Merge pull request #10357 from jfrazelle/bash-completion-stats
f23c136 
Add completion for stats.
@icecrime icecrime merged commit f23c136 into moby:master Jan 28, 2015
@jessfraz jessfraz deleted the bash-completion-stats branch January 28, 2015 18:35
@SvenDowideit SvenDowideit mentioned this pull request Feb 9, 2015
Closed
@SvenDowideit SvenDowideit mentioned this pull request Feb 26, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.5.0
Development

Successfully merging this pull request may close these issues.
3 participants
@jessfraz @tianon @icecrime
0