Skip to content

use table output for better user experience#130

Merged
dbsid merged 2 commits intopingcap:masterfrom
sillydong:update_output_to_table
Aug 16, 2022
Merged

use table output for better user experience#130
dbsid merged 2 commits intopingcap:masterfrom
sillydong:update_output_to_table

Conversation

@sillydong
Copy link
Copy Markdown
Contributor

@sillydong sillydong commented Aug 7, 2022

Hi all, I'm working on this PR to give a try for outputting the result as table style. Originally, the output is a line and not aligned. With table style output, we can get better user experience and it can be much more convenient to copy values.

The output would like this:
image

in comparison, the original output is like this:
image

This is a simple try and I have not written it in a better way. I would first like to get some feedback on this approach and later I can make it better.

Any feedback is welcomed.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Aug 7, 2022

CLA assistant check
All committers have signed the CLA.

@sillydong
Copy link
Copy Markdown
Contributor Author

Beside this approach with tablewriter, we can also do some manual alignment with fmt.Sprintf and fixed string size to make it better. But that's a little more inconvenient. What do you think?

@siddontang
Copy link
Copy Markdown
Member

cool, maybe it is better to provide an output format flag, like table or origin raw message? so we can further support JSON, etc.

Another thing maybe we already used some scripts to parse the output results in our CI, so here we need to ensure the compatibility.

@siddontang
Copy link
Copy Markdown
Member

maybe we can also support this in https://github.com/pingcap/go-ycsb

@sillydong can you have a try later?

@sillydong
Copy link
Copy Markdown
Contributor Author

Seems like they are similar tools. If this approach has no problem, I will make it more formal and update this PR.
After we merge this PR I can do it again in go-ycsb.

@sillydong
Copy link
Copy Markdown
Contributor Author

cool, maybe it is better to provide an output format flag, like table or origin raw message? so we can further support JSON, etc.

I will try adding a flag output to control the output style, available values may be table,plaintext,json, the default will be table for human read. Is this OK?

Another thing maybe we already used some scripts to parse the output results in our CI, so here we need to ensure the compatibility.

Could you share more detail about the CI? I didn't find any thing for that, only a test case in hist_test.go. Is there anything more?

@siddontang
Copy link
Copy Markdown
Member

@sillydong

I will try adding a flag output to control the output style, available values may be table,plaintext,json, the default will be table for human read. Is this OK?

Fine, but IMO I prefer plain instead of plaintext :-)

For CI, means PingCAP's internal CI or bench system, has not been public.

@dbsid
Copy link
Copy Markdown
Collaborator

dbsid commented Aug 8, 2022

cool, maybe it is better to provide an output format flag, like table or origin raw message? so we can further support JSON, etc.

I will try adding a flag output to control the output style, available values may be table,plaintext,json, the default will be table for human read. Is this OK?

Another thing maybe we already used some scripts to parse the output results in our CI, so here we need to ensure the compatibility.

Could you share more detail about the CI? I didn't find any thing for that, only a test case in hist_test.go. Is there anything more?

Inside PingCap, we've scripts to interpret the go-tpc result based on the current output format, please make the current format as the default for compatibility.

@sillydong
Copy link
Copy Markdown
Contributor Author

OK, the value would be plain,table,json, and default is plain.

For CI part, you may add cases for new formats after this.

@siddontang
Copy link
Copy Markdown
Member

any update? @sillydong

@sillydong
Copy link
Copy Markdown
Contributor Author

not yet, will update tonight

@sillydong sillydong force-pushed the update_output_to_table branch from aab2079 to ba7c642 Compare August 11, 2022 15:45
@sillydong
Copy link
Copy Markdown
Contributor Author

sillydong commented Aug 11, 2022

command help info is like this
image

here are some snapshots for output

plain
image

table
image

json
image

@dbsid dbsid self-requested a review August 11, 2022 23:15
@sillydong sillydong force-pushed the update_output_to_table branch from ea81139 to d31d20a Compare August 16, 2022 14:31
@sillydong
Copy link
Copy Markdown
Contributor Author

updated the go.mod

@dbsid dbsid merged commit 062d2d7 into pingcap:master Aug 16, 2022
@siddontang
Copy link
Copy Markdown
Member

thanks @sillydong

can you do the same thing for go-ycsb?

@sillydong
Copy link
Copy Markdown
Contributor Author

working on learning the detail of go-ycsb, may take some time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants