-
Notifications
You must be signed in to change notification settings - Fork 380
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
[#5057] Added first part of CLI code #5058
Conversation
clients/cli/src/main/java/org/apache/gravitino/cli/commands/CatalogDetails.java
Outdated
Show resolved
Hide resolved
clients/cli/src/main/java/org/apache/gravitino/cli/FullName.java
Outdated
Show resolved
Hide resolved
clients/cli/src/main/java/org/apache/gravitino/cli/FullName.java
Outdated
Show resolved
Hide resolved
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java
Outdated
Show resolved
Hide resolved
@shaofengshi @diqiu50 @yuqi1129 @mchades if you could review agian that would be appreciated |
Hi Justin, my comments are all resolved now, thank you! I see you have replied Jerry's comments; I think he suggests to move the document to gravitino project's "docs" folder (not in cli module) and make its style consistent with others; besides, the error.sh and example.sh are sample commands, not for execution, which can be a part of documentation? You'd better double confirm with him. |
Yes, docs should be in the "docs" folder, and it is user-facing doc, so any other content like test/contribute is not necessary to add. Besides, these two shell scripts are just for demo purposes, it is not so useful and can be covered by doc, I suggest that you remove these two scripts here. You can add a script for running cli easily, like "gravitino.sh", for example "gcli.sh", this can be done in the later PR, but it should be in the "bin" folder. |
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java
Show resolved
Hide resolved
clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java
Outdated
Show resolved
Hide resolved
clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java
Outdated
Show resolved
Hide resolved
clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListMetalakes.java
Show resolved
Hide resolved
clients/cli/src/main/java/org/apache/gravitino/cli/commands/TableDetails.java
Show resolved
Hide resolved
All requested changes have been done, @jerryshao if you could please review, that would be appreciated. |
### What changes were proposed in this pull request? Added initial CLI to list information about metalakes, catalogs, schema and tables. ### Why are the changes needed? For a CLI. Fix: apache#5057 ### Does this PR introduce _any_ user-facing change? No, but it add a new API. ### How was this patch tested? Unit test pass locally.
What changes were proposed in this pull request?
Added initial CLI to list information about metalakes, catalogs, schema and tables.
Why are the changes needed?
For a CLI.
Fix: #5057
Does this PR introduce any user-facing change?
No, but it add a new API.
How was this patch tested?
Unit test pass locally.