-
Notifications
You must be signed in to change notification settings - Fork 406
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
[#6496] feat(CLI): Support plain format output for Schema and Table command #6497
[#6496] feat(CLI): Support plain format output for Schema and Table command #6497
Conversation
@justinmclean @tengqm could you please review this PR when you have time? I’d really appreciate your feedback. |
It looks good to me, except for one minor thing with the output. There should be no spaces after the comma. The output is basically CSV, and we should not add extra spaces. |
What's the use case for the CSV output? |
It's not so much a use case for CSV. This change changes the output and could break user scripts using our CLI. |
Similar to the table output PR there's no real need to call loadTable/loadSchema |
Even if we have a use case for this, we'll need a design for the output. We NEVER do something simply because it is DOABLE. There are simply a thousand ways to spend your precious time. |
@tengqm The main thing is that we can't use web UI on our side, and users are not professional and technical personnel. There is no need to use Java API or REST API to create and delete tables for now. We need to provide a simple and practical client-side script for non-technical people to query(just list\ details). |
Okay, we can support tabular output for command line users, where the most important fields are included. For cases where there are too many fields, we can simply dump the JSON response. So ... I don't see a need for CSV output. Am I missing something? |
@tengqm @justinmclean Yes, just to be consistent with the tableformat output logic, Whether the plainformat and tableformat outputs are inconsistent for the same entity |
If people want JSON output they can use the REST interface. JSON output was discarded very early on as an aim for the CLI. |
Good to know. |
Yes the REST interface can be interacted with using |
@justinmclean should the plainformat and tableformat outputs are inconsistent for the same entity? or plainformat show different messages. |
@Abyss-lord I'm not sure what you mean here, can you point to an example? |
…able command Support plain format output for Schema and Table command
…able command fix some bugs.
@justinmclean in table format table details will return as follows: +----------+---------+---------------+----------+---------+
| Name | Type | AutoIncrement | Nullable | Comment |
+----------+---------+---------------+----------+---------+
| id | integer | false | true | N/A |
| name | string | false | true | N/A |
| standard | long | false | true | N/A |
| dt | string | false | true | N/A |
+----------+---------+---------------+----------+---------+ in plain format, it should return as same as table format or just return |
The original object was that a LIST command should give just the basic information, i.e. name and comment, and a DETAILS command would provide more details. They should be consistent across both table and plain formats. |
…able command fix some bugs.
36822fd
to
b587c72
Compare
@justinmclean yes Justin , |
Have the space after coma's been removed in the plain output? |
clients/cli/src/main/java/org/apache/gravitino/cli/outputs/PlainFormat.java
Show resolved
Hide resolved
…able command fix COMMA_JOINER.
@justinmclean The COMMA JOINER has removed the Spaces. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution.
What changes were proposed in this pull request?
Support plain format output for Schema and Table command
Why are the changes needed?
Fix: #6496
Does this PR introduce any user-facing change?
No
How was this patch tested?
local test