[DOCFIX] Define CLI conventions in README #17530
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Automated checks report:
Some checks failed. Please fix the reported issues and reply 'alluxio-bot, check this please' to re-run checks. |
Xenorith
changed the title
|
Automated checks report:
All checks passed! |
Xenorith
commented
Jun 1, 2023
| ## Layout and naming conventions | ||
|
|
||
| The choice of names for services, operations, and flags should be succinct: short and unambiguous. | ||
| Use of a single word is strongly preferred, but otherwise the name parts should be delimited by a dash such as `foo-bar`. |
There was a problem hiding this comment.
wonder if it would be appropriate to declare that flags should be --foo-bar but values should be fooBar. on one hand, it's more obvious what is a value or argument vs a flag name. on the other hand, it mixes up the casing format.
There was a problem hiding this comment.
That's a good question. I'm partial to foo-bar for both because I don't like typing capital letters in a cli, but it would help visually differentiate them.
There was a problem hiding this comment.
actually i take that back, we generally don't have a choice in the value's format. that's really up to the underlying implementation...
| bin/cli.sh item mark --unset name | ||
| ``` | ||
| where it is expected that either `--set` or `--unset` are specified. | ||
| This is preferred over the alternative of two separate commands with `setMark` and `unsetMark` as the operations. |
There was a problem hiding this comment.
it is not very clear to me what the rule or guideline is, this still seem pretty subjective. I think we should mention the pattern and format. preferably lay it out like this for people to follow / understand. what do you think?
https://primer.style/design/native/cli/foundations
There was a problem hiding this comment.
Commands follow the format of:
bin/cli.sh <service> <operation> [--<flag>[=<value>]] [<args>]
There was a problem hiding this comment.
It could be a good idea to link the primer.style website to help people understand where the guidelines are coming from and provide further examples of the style to use.
There was a problem hiding this comment.
i can link it as a reference/inspiration but not refer to it in a way that implies we are following it because we're not -> that'd cause confusion
jenoudet
approved these changes
Jun 2, 2023
|
alluxio-bot, merge this please |
|
merge failed: |
|
alluxio-bot, merge this please |
twalluxio
pushed a commit
to twalluxio/alluxio
that referenced
this pull request
Aug 14, 2023
dev docs for defining conventions for the new alluxio cli in golang see Alluxio#17522 pr-link: Alluxio#17530 change-id: cid-12886237cf2135b3a8c37d6b0ffa01ff6aaa6d92
Xenorith
added a commit
that referenced
this pull request
Aug 15, 2023
- Add cli/ folder containing golang code - Add script to compile code to executable in build/cli/build-cli.sh - Add entrypoint script for development in bin/cli.sh - Add build profile to build the CLI as part of tha maven build via `-PgoCli` - Add `conf` service, consisting of - `bin/cli.sh conf get` as an example command, equivalent to `bin/alluxio getConf` - `bin/cli.sh conf log --name fully.qualified.class.path`, equivalent to `bin/alluxio fsadmin logLevel --logName fully.qualified.class.path` See #17522 for more background info pr-link: #17532 dev docs for defining conventions for the new alluxio cli in golang pr-link: #17530 - Add commands to start/stop individual processes, ex. `bin/cli.sh process start master` - Define process interface and registry - process specific java opts are defined with the process and are dynamically added to env - Add journal format command as part of starting master process - Mount options for worker are not ported because they will be deprecated in the near future pr-link: #17561 Adds `info` subcommand, which contains: - `cache`: worker capacity information, calls the existing `bin/alluxio fsadmin report capacity` command - `collect`: collects cluster information into a single tarball, calls the existing `bin/alluxio collectInfo` command - `master`: master quorum information, calls the existing `bin/alluxio fs masterInfo` command - `report`: cluster information, calls the existing `bin/alluxio fsadmin report` command, excluding the `capacity` category pr-link: #17566 Add journal commands to CLI as part of #17522 pr-link: #17569 Add quorum/HA related commands as part of #17522 pr-link: #17570 Add fs commands to golang CLI as part of #17522 unlike other CLI commands, utilize arguments to maintain the existing structure of filesystem commands (ex. cp, du, ls, mv, rm, etc) pr-link: #17580 Add generate commands to golang CLI as part of #17522 bin/alluxio docGen -> bin/cli generate docs bin/alluxio bootstrapConf -> bin/cli generate template to read ALLUXIO_CONF_DIR, also conducted a minor change on env variables. pr-link: #17790 Add exec commands to golang CLI as part of #17522 `bin/alluxio runClass` -> `bin/cli exec class` `bin/alluxio runTests` -> `bin/cli exec testRun` `bin/alluxio runUfsTests` -> `bin/cli exec testUfs` `bin/alluxio runUfsIOTest` -> `bin/cli exec testUfsIO` `bin/alluxio runHdfsMountTests` -> `bin/cli exec testHdfsMount` `bin/alluxio runHmsTests` -> `bin/cli exec testHms` `bin/alluxio runJournalCrashTest` -> `bin/cli exec testJournalCrash` For command `exec test`, using different `--name` flags can lead to huge difference in other flags and options. To manage flags and options, currently using different commands for different test types. pr-link: #17797 Prepare golang cli for tarball build and replace bin/alluxio allow java 8 or 11 add cli binaries to other tarball profiles Add `process` commands with multiple nodes to golang CLI as part of #17522 `bin/alluxio-start.sh masters` -> `bin/cli.sh process start masters` `bin/alluxio-start.sh job_masters` -> `bin/cli.sh process start job_masters` `bin/alluxio-start.sh workers` -> `bin/cli.sh process start workers` `bin/alluxio-start.sh job_workers` -> `bin/cli.sh process start job_workers` `bin/alluxio-start.sh proxies` -> `bin/cli.sh process start proxies` `bin/alluxio-start.sh all` -> `bin/cli.sh process start all` `bin/alluxio-start.sh local` -> `bin/cli.sh process start local` `bin/alluxio-stop.sh masters` -> `bin/cli.sh process stop masters` `bin/alluxio-stop.sh job_masters` -> `bin/cli.sh process stop job_masters` `bin/alluxio-stop.sh workers` -> `bin/cli.sh process stop workers` `bin/alluxio-stop.sh job_workers` -> `bin/cli.sh process stop job_workers` `bin/alluxio-stop.sh proxies` -> `bin/cli.sh process stop proxies` `bin/alluxio-stop.sh all` -> `bin/cli.sh process stop all` `bin/alluxio-stop.sh local` -> `bin/cli.sh process stop local` For command `process start/stop` on multiple nodes, using crypto's `ssh` package to create an SSH session, connect to masters, workers or all nodes, then send according subcommand on single nodes to these nodes. pr-link: #17887 Add job commands to golang CLI as part of #17522 `bin/alluxio-bash job cancel id` -> `bin/alluxio job cancel --id` `bin/alluxio-bash job leader` -> `bin/alluxio job leader` `bin/alluxio-bash job ls` -> `bin/alluxio job list` `bin/alluxio-bash job getCmdStatus jobControlId` -> `bin/alluxio job cmdStatus --id` `bin/alluxio-bash job stat [-v] id` -> `bin/alluxio job jobStatus [-v] --id` `bin/alluxio-bash fs distributedCp src dst` -> `bin/alluxio job submit --type cp --src --dst` `bin/alluxio-bash fs distributedMv src dst` -> `bin/alluxio job submit --type mv --src --dst` `bin/alluxio-bash fs load path --submit` -> `bin/alluxio job load --path` pr-link: #17931 Clean up multiprocess code - refactor commands launching the same process on multiple hosts into a single struct - simplify the logic to ssh into different hosts with more straightforward handling of error channel - move names.go under its own package to avoid import cycles pr-link: #17947 Add an `info version` subcommand to golang CLI as part of #17522 `bin/alluxio-bash version` -> `bin/alluxio info version` pr-link: #17943 update calls to old bash scripts to refer to alluxio-bash Add `cache` commands to golang CLI as part of #17522 `bin/alluxio-bash formatWorker` -> `bin/cli cache format` `bin/alluxio-bash fs freeWorker` -> `bin/cli cache free --worker` `bin/alluxio-bash fs free` -> `bin/cli cache free [-f] --path` `bin/alluxio-bash fs persist` -> `bin/cli cache persist --path` `bin/alluxio-bash fs setTtl` -> `bin/cli cache ttl --set --duration --action` `bin/alluxio-bash fs unsetTtl` -> `bin/cli cache ttl --unset` pr-link: #17937 backcompat support for start/stop scripts skip mount args in start script backcompat remove unsupported cmds
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
dev docs for defining conventions for the new alluxio cli in golang
see #17522