[#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. |
justinmclean
left a comment
There was a problem hiding this comment.
Thanks for the contribution.
…able command (apache#6497) ### What changes were proposed in this pull request? Support plain format output for Schema and Table command ### Why are the changes needed? Fix: apache#6496 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? local test ```bash gcli schema list -m demo_metalake --name Hive_catalog -i default,Default Hive database gcli table list -m demo_metalake --name Hive_catalog.default -i employee_partitions source_table test_dt_partition test_key_en sales order_tb1 tmp float_test test_tbl gcli table details -m demo_metalake --name Hive_catalog.default.test1 -i test1,N/A gcli schema details -m demo_metalake --name Hive_catalog.default -i default,Default Hive database ```
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