[#6148] improve(CLI): Refactor model commands in Gavitino CLI #6162
Conversation
Refactor model commands and Base class in Gavitino CLI.
|
Hi @justinmclean @tengqm , could you please review this PR when you have time? I’d really appreciate your feedback. |
| this.metalake = name.getMetalakeName(); | ||
| this.catalog = name.getCatalogName(); | ||
| this.schema = name.getSchemaName(); | ||
| this.model = name.getModelName(); |
There was a problem hiding this comment.
This is likely to cause an issue with the list command.
There was a problem hiding this comment.
getModelName will return null if names.length <= position， list command is executed first, IMHO, It should be retrieved in the constructor.
There was a problem hiding this comment.
Even so, you should not call it if it is not needed. It will put out an error message saying the name is malformed - we don't want this as it will confuse the user. I'd remove the final and set it after the list command call.
clients/cli/src/main/java/org/apache/gravitino/cli/ModelCommandHandler.java
Outdated
Show resolved
Hide resolved
|
Hi @justinmclean , I’ve finished updating the code. Please take a look at the PR again when you have time. |
…pache#6162) ### What changes were proposed in this pull request? Refactor model commands and Base class in Gavitino CLI. ### Why are the changes needed? Fix: apache#6148 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? local test
What changes were proposed in this pull request?
Refactor model commands and Base class in Gavitino CLI.
Why are the changes needed?
Fix: #6148
Does this PR introduce any user-facing change?
No
How was this patch tested?
local test