feature: add --format option for pouch ps #2844

aohang111 · 2019-05-14T07:35:40Z

Ⅰ. Describe what this PR did

for the issue #2827. CLI compatibility between Pouch and Docker

solve the pouch ps --format option.
if other command(such as pouch history/images/network/volume/stats/version) also need --format option.
can done similarly

Ⅱ. Does this pull request fix one issue?

fixes #2827,

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

have add unit test and integration test

Ⅳ. Describe how to verify it

root@ahsh-main:~/go/src/github.com/alibaba/pouch# pouch ps
Name ID Status Created Image Runtime
58b7de 58b7de Up 40 minutes 40 minutes ago registry.hub.docker.com/library/ubuntu:latest runc

root@ahsh-main:~/go/src/github.com/alibaba/pouch# pouch ps -a --format "table {{.ID}}\t{{.Status}}\t{{.Created}}"
ID Status Created
58b7de Up 11 seconds 11 seconds ago

root@ahsh-main:~/go/src/github.com/alibaba/pouch# pouch ps -a --format "table {{.ID}}\t{{.Status}}\t{{.Created}}\t{{.Name}}"
ID Status Created Name
58b7de Up 40 minutes 40 minutes ago 58b7de

root@ahsh-main:~/go/src/github.com/alibaba/pouch# pouch ps -a --format "raw"
Name:58b7de
ID:58b7de
Status:Up 4 seconds
Created:4 seconds ago
Image:registry.hub.docker.com/library/ubuntu:latest
Runtime:runc

Ⅴ. Special notes for reviews

with @houstar's help.

codecov · 2019-05-14T07:35:43Z

Codecov Report

Merging #2844 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2844      +/-   ##
==========================================
- Coverage   69.09%   69.05%   -0.04%     
==========================================
  Files         285      285              
  Lines       17844    17844              
==========================================
- Hits        12330    12323       -7     
- Misses       4114     4122       +8     
+ Partials     1400     1399       -1

Flag	Coverage Δ
criv1alpha2_test	`38.97% <ø> (-0.04%)`	⬇️
integration_test_0	`36.62% <ø> (+0.05%)`	⬆️
integration_test_1	`35.33% <ø> (-0.43%)`	⬇️
integration_test_2	`36.57% <ø> (+0.10%)`	⬆️
integration_test_3	`35.39% <ø> (-0.07%)`	⬇️
node_e2e_test	`34.81% <ø> (+0.03%)`	⬆️
unittest	`28.53% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/streams/utils.go	`82.14% <0.00%> (-7.15%)`	⬇️
ctrd/watch.go	`72.97% <0.00%> (-5.41%)`	⬇️
pkg/meta/store.go	`67.44% <0.00%> (-1.56%)`	⬇️
daemon/mgr/container_utils.go	`76.76% <0.00%> (-1.02%)`	⬇️
daemon/mgr/container.go	`59.72% <0.00%> (-0.63%)`	⬇️
ctrd/container.go	`54.30% <0.00%> (+0.38%)`	⬆️
cri/v1alpha2/cri.go	`69.57% <0.00%> (+0.50%)`	⬆️
daemon/mgr/spec_linux.go	`80.21% <0.00%> (+1.06%)`	⬆️
daemon/containerio/io.go	`74.75% <0.00%> (+1.94%)`	⬆️
daemon/logger/crilog/log.go	`82.60% <0.00%> (+2.89%)`	⬆️

pouchrobot · 2019-05-14T07:35:48Z

We found this is your first time to contribute to Pouch, @aohang111
👏 We really appreciate it.
Just remind that you have read the contribution guide: https://github.com/alibaba/pouch/blob/master/CONTRIBUTING.md
If you didn't, you should do that first. If done, welcome again and please enjoy hacking! 🍻

CLAassistant · 2019-05-14T07:35:48Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

houstar · 2019-05-14T07:39:29Z

@aohang111 Could you use gofmt -s to format your source code as officially style? thanks

allencloud · 2019-05-15T03:45:17Z

Thank you greatly for your work. @aohang111
While I am wondering if we could add integration test for the pull request, not only the unit test.
Please feel free to tell us if you have some questions. Thanks again.

houstar · 2019-05-15T05:07:38Z

Could you squash previous four commits into one commits ?

aohang111 · 2019-05-15T08:39:01Z

@allencloud
I am trying to add the integration test. by the way.
@houstar
will squash the commits into one commits.thanks

aohang111 · 2019-05-16T07:28:39Z

@allencloud ,I have added the integration test. please assign someone to review this code ASAP
@houstar thanks for your guide and help

houstar · 2019-05-16T07:46:34Z

Well done. @aohang111 You've made this happen as your first contribution to pouch project

cli/cli.go

cli/formatter/container.go

cli/formatter/formatter_test.go

cli/formatter/formatter.go

cli/formatter/container.go

cli/formatter/formatter.go

cli/formatter/container.go

aohang111 · 2019-05-20T05:21:50Z

@ZYecho ,modified and comments. pls review. thanks

cli/formatter/container.go

cli/formatter/formatter.go

ZYecho · 2019-05-20T06:53:10Z

@ZYecho ,modified and comments. pls review. thanks

Thanks for your contribution, just minor suggestions. Good Job!

aohang111 · 2019-05-20T10:48:28Z

@ZYecho commit again. pls review. thanks

test/cli_ps_test.go

cli/formatter/formatter.go

aohang111 · 2019-05-21T03:19:28Z

@ZYecho .done.

pouchrobot · 2019-10-26T13:31:14Z

ping @aohang111
Conflict happens after merging a previous commit.
Please rebase the branch against master and push it back again. Thanks a lot.

cxz66666 · 2022-07-08T07:59:51Z

@fuweid ,if the user specify the quite mode. we just need to output the shorted container id . no need to care about if the format is specified. so I thought it is split with the logic of format display. what's your opinion .

I also think it's needless to care about the output format in quite mode, please resolve some conflicts ASAP and latter it will be merged into master!

cxz66666 · 2022-07-08T07:15:15Z

cli/cli.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"strconv"
 	"text/tabwriter"
+	"text/template"


There will be a merge conflict, please handle and rebase it!

cxz66666 · 2022-07-08T07:28:36Z

cli/formatter/container.go

+	if flagNoTrunc {
+		id = c.ID
+	}
+	runningFor, err := utils.FormatTimeInterval(c.Created)


utils.FormatTimeInterval function have an extra new param, please fix it!

cxz66666 · 2022-07-08T07:37:29Z

cli/ps.go

 	for _, c := range containers {
-		created, err := utils.FormatTimeInterval(c.Created)
+		containerContext, err := formatter.NewContainerContext(c, p.flagNoTrunc)


There is a merge conflict, please handle it!

cxz66666 · 2022-07-08T07:43:26Z

cli/ps.go

+	format = formatter.PreFormat(format)
+	if tableOrRaw {
+		containerHeader := formatter.ContainerHeader
+		p.cli.FormatDisplay(format, tmplH, containerHeader, w)


Should we handle the possible error at here and follow?
In my opinion, we should care about possible error during parsing the template, at least print some error messages.

cxz66666 · 2022-07-08T08:01:48Z

cli/ps.go

 	}
-	display.Flush()
+	w.Flush()


please use _=w.Flush() instead of it!

cxz66666 · 2022-07-18T08:18:30Z

Thank you for your contribution! because you don't response for a long time, I will close this PR and reopen #3037. ❤️

pouchrobot added the size/XL label May 14, 2019

aohang111 changed the title ~~Issue 2827~~ add --format option for pouch ps May 14, 2019

allencloud added areas/cli kind/feature labels May 16, 2019

aohang111 force-pushed the issue_2827 branch 3 times, most recently from 1fec4f7 to 10c7c29 Compare May 16, 2019 06:58

allencloud assigned ZYecho May 16, 2019

ZYecho reviewed May 19, 2019

View reviewed changes

weekly-digest bot mentioned this pull request May 19, 2019

Weekly Digest (12 May, 2019 - 19 May, 2019) #2854

Closed

aohang111 force-pushed the issue_2827 branch from 10c7c29 to e29fed0 Compare May 20, 2019 04:50

ZYecho reviewed May 20, 2019

View reviewed changes

aohang111 force-pushed the issue_2827 branch 2 times, most recently from 5f7d9ee to 00922cc Compare May 20, 2019 10:01

ZYecho reviewed May 21, 2019

View reviewed changes

test/cli_ps_test.go Show resolved Hide resolved

ZYecho reviewed May 21, 2019

View reviewed changes

cli/formatter/formatter.go Show resolved Hide resolved

aohang111 force-pushed the issue_2827 branch from 00922cc to b29b7a2 Compare May 21, 2019 03:06

weekly-digest bot mentioned this pull request Sep 22, 2019

Weekly Digest (15 September, 2019 - 22 September, 2019) #2974

Closed

weekly-digest bot mentioned this pull request Oct 6, 2019

Weekly Digest (29 September, 2019 - 6 October, 2019) #2977

Closed

pouchrobot added conflict/needs-rebase and removed conflict/needs-rebase labels Oct 11, 2019

weekly-digest bot mentioned this pull request Oct 13, 2019

Weekly Digest (6 October, 2019 - 13 October, 2019) #2983

Closed

pouchrobot added conflict/needs-rebase and removed conflict/needs-rebase labels Oct 14, 2019

pouchrobot added conflict/needs-rebase and removed conflict/needs-rebase labels Oct 21, 2019

cardyok requested a review from cxz66666 July 7, 2022 12:15

cxz66666 requested changes Jul 8, 2022

View reviewed changes

cxz66666 mentioned this pull request Jul 18, 2022

[feat]: add format option for pouch ps and integrate/unit test #3037

Merged

cxz66666 closed this Jul 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: add --format option for pouch ps #2844

feature: add --format option for pouch ps #2844

aohang111 commented May 14, 2019 •

edited

Loading

codecov bot commented May 14, 2019 •

edited

Loading

pouchrobot commented May 14, 2019

CLAassistant commented May 14, 2019 •

edited

Loading

houstar commented May 14, 2019

allencloud commented May 15, 2019

houstar commented May 15, 2019

aohang111 commented May 15, 2019 •

edited

Loading

aohang111 commented May 16, 2019 •

edited

Loading

houstar commented May 16, 2019

aohang111 commented May 20, 2019

ZYecho commented May 20, 2019

aohang111 commented May 20, 2019

aohang111 commented May 21, 2019

pouchrobot commented Oct 26, 2019

cxz66666 commented Jul 8, 2022

cxz66666 Jul 8, 2022

cxz66666 Jul 8, 2022

cxz66666 Jul 8, 2022

cxz66666 Jul 8, 2022

cxz66666 Jul 8, 2022

cxz66666 commented Jul 18, 2022

feature: add --format option for pouch ps #2844

feature: add --format option for pouch ps #2844

Conversation

aohang111 commented May 14, 2019 • edited Loading

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

codecov bot commented May 14, 2019 • edited Loading

Codecov Report

pouchrobot commented May 14, 2019

CLAassistant commented May 14, 2019 • edited Loading

houstar commented May 14, 2019

allencloud commented May 15, 2019

houstar commented May 15, 2019

aohang111 commented May 15, 2019 • edited Loading

aohang111 commented May 16, 2019 • edited Loading

houstar commented May 16, 2019

aohang111 commented May 20, 2019

ZYecho commented May 20, 2019

aohang111 commented May 20, 2019

aohang111 commented May 21, 2019

pouchrobot commented Oct 26, 2019

cxz66666 commented Jul 8, 2022

cxz66666 Jul 8, 2022

Choose a reason for hiding this comment

cxz66666 Jul 8, 2022

Choose a reason for hiding this comment

cxz66666 Jul 8, 2022

Choose a reason for hiding this comment

cxz66666 Jul 8, 2022

Choose a reason for hiding this comment

cxz66666 Jul 8, 2022

Choose a reason for hiding this comment

cxz66666 commented Jul 18, 2022

aohang111 commented May 14, 2019 •

edited

Loading

codecov bot commented May 14, 2019 •

edited

Loading

CLAassistant commented May 14, 2019 •

edited

Loading

aohang111 commented May 15, 2019 •

edited

Loading

aohang111 commented May 16, 2019 •

edited

Loading