Skip to content
This repository has been archived by the owner on Sep 22, 2023. It is now read-only.

refactor: Make CLI commands consistent #163

Merged
merged 49 commits into from Aug 31, 2021
Merged

Conversation

achimnol
Copy link
Member

@achimnol achimnol commented Jun 22, 2021

Let's make command arguments and options to be consistent and update to Click 8.0.x.

The client SDK consists of: low-level API (auth, request) + functional API (wrappers around REST and GraphQL) + CLI commands. This PR will rewrite the CLI commands, and this will not be backward compatible due to ambiguity of command parsing. (Just imagine that etcdctl requires ETCDCTL_API environment variable to switch between v2 and v3 command sets.)

What's included in this PR:

  • API v4 or older support is removed with an explicit warning during version negotiation.
  • Python 3.7 support is removed and now it requires 3.8 or later.
  • New consistent CLI command hierarchy with backend.ai admin? <entity> {info,list,...}. e.g.,
    • Before
      • backend.ai admin agents
      • backend.ai admin agent i-dgx001 (😞 mix of plural and singular "agent" and "agents")
      • backend.ai admin scaling-group -n default (😞 some individual item query commands just accept an argument but some accept an arbitrarily named option -- inconsistent!!)
    • After
      • backend.ai admin agent list
      • backend.ai admin agent info i-dgx001 (💯 now consistent singular name followed by list, info)
      • backend.ai admin scaling-group info default (💯 now consistently accept the key as a sole argument)
    • Still it preserves many frequently-used shortcuts such as run, ps, rm, etc., but they are now the pure duplicate, by generating the command handler function twice for different command groups and thus removing potential of missing new options/arguments due to manual copy-and-paste of codes.
  • Global --output=console (the default) or --output=json option to control the output format of CLI list and info commands. e.g.,
    • backend.ai --output=json ps --dead
    • backend.ai --output=json admin agent list
    • This introduces a new "output" framework as ai.backend.client.output subpackage.
      • Now all functional list/detail APIs must be called with a list of FieldSpec as fields arguments, instead of a list of str.
      • You may refer the predefined FieldSpec instances from ai.backend.client.output.fields and each functional API module's _default_list_fields and _default_detail_fields variables.
      • It standardizes the error output and exit code so that users may write more sophisticated automation scripts using the CLI with jq command and shell scripts.
  • --limit and --offset to control pagination of paginated list commands (NOTE: Not all list commands support this...)
    • e.g., backend.ai ps --dead --offset=30 --limit=20
    • All JSON output of list commands now include count (the number of items in the current page result), total_count (the number of all items in the server), items (the list of item objects) fields so that client may perform its own pagination as needed, in a consistent way.

Left as future work:

  • This PR does not implement the --output=json support for mutation and session execution commands. They currently work like before, just printing out console messages regardless of the output option.

* Move individual "entity" and "entities" commands
  into "entity info" and "entity list" commands.
@codecov
Copy link

codecov bot commented Jun 22, 2021

Codecov Report

Merging #163 (87271cb) into main (c86d588) will increase coverage by 3.41%.
The diff coverage is 53.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #163      +/-   ##
==========================================
+ Coverage   42.06%   45.47%   +3.41%     
==========================================
  Files          68       74       +6     
  Lines        6652     6793     +141     
==========================================
+ Hits         2798     3089     +291     
+ Misses       3854     3704     -150     
Impacted Files Coverage Δ
src/ai/backend/client/exceptions.py 78.57% <ø> (-1.43%) ⬇️
src/ai/backend/client/cli/announcement.py 28.57% <20.00%> (ø)
src/ai/backend/client/output/console.py 21.91% <21.91%> (ø)
src/ai/backend/client/output/formatters.py 31.32% <31.32%> (ø)
src/ai/backend/client/cli/admin/session.py 39.02% <39.02%> (ø)
src/ai/backend/client/cli/params.py 39.47% <39.47%> (ø)
src/ai/backend/client/cli/session.py 43.27% <43.27%> (ø)
src/ai/backend/client/cli/admin/agent.py 47.61% <47.61%> (ø)
src/ai/backend/client/pagination.py 28.57% <50.00%> (+3.57%) ⬆️
src/ai/backend/client/cli/admin/group.py 36.02% <52.77%> (ø)
... and 68 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c86d588...87271cb. Read the comment docs.

* No longer duplicate arguments and options
* Just "generate" the command functions multiple times and place
  it under different Click groups.
* Avoid circular imports by making client.cli.main as a module
  when doing so.
* All GraphQL/output fields are now defined as FieldSpec.
* All CLI/functional API implementations refer the central FieldSet
  defined in the `ai.backend.client.output.fields` module.
* All FieldSpec instances have their own formatters.
@CLAassistant
Copy link

CLAassistant commented Aug 31, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ nokchalatte
✅ achimnol
❌ Chrysan Angela Piarso


Chrysan Angela Piarso seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@achimnol
Copy link
Member Author

The CLA check failure is due to a misconfiguraion of git client: chrysan@Chrysans-MacBook-Pro.local, which is not a real email address. It's the same person to nokchalatte, so I'm overriding the CLA check with the admin privilege.

* Create and apply `sizebytes_output_formatter`
@achimnol achimnol merged commit be7c174 into main Aug 31, 2021
@achimnol achimnol deleted the feature/revamp-cli-consistency branch August 31, 2021 12:27
youngjun0627 pushed a commit to youngjun0627/backend.ai-client-py that referenced this pull request Sep 12, 2021
* refactor: Make all cli.admin module names singular
* refactor: Reorganize admin command hierarchy
  - Move individual "entity" and "entities" commands
    into "entity info" and "entity list" commands.
  - No longer duplicate arguments and options
  - Just "generate" the command functions multiple times and place
    it under different Click groups.
  - Avoid circular imports by making client.cli.main as a module
    when doing so.

* wip: Add `client.cli.output` subpkg to provide common interface for output
  - All GraphQL/output fields are now defined as FieldSpec.
  - All CLI/functional API implementations refer the central FieldSet
    defined in the `ai.backend.client.output.fields` module.
  - All FieldSpec instances have their own formatters.

* setup: Update backend.ai-cli
* fix: remove support for servers with API ver older than v5.20191215
* maintenance: Remove Python 3.7 support and now require 3.8 or later.
* fix: Improve handling of JSON-string CLI args better

* Overrided the CLA check due to git client misconfiguration to specify author emails

Co-authored-by: Chrysan Angela Piarso <chrysan@lablup.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants