-
Notifications
You must be signed in to change notification settings - Fork 220
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
add cli subcommands #547
base: main
Are you sure you want to change the base?
add cli subcommands #547
Conversation
Now it prints pool's users:
|
Now it prints users, shards and servers. I also work on to save some screen space by shorting some column's names.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is super cool!
Let me know if you'd like me to merge this as is, or you'd like to keep working on it.
Just one suggestion:
Commands:
pools Manage pool's configuration
users not implemented
shards not implemented
help
Both users
and shards
, as far as I understand their purpose here, would only make sense in the context of a specific pool. So I think they should be subcommands of pools
, maybe something like pools edit
or pools shards add
, pools users remove
, etc.
More things I think would be cool to implement as well:
- show the values from
General
config, e.g. timeouts, port, host, etc. - limit how many pools are listed, with a search function; some configs in multi-tenant situations can get very long.
Nice! Thank you!
Let me clean the code a bit and removing the "not implemented" things then I'll poke when I'm done.
Yes. It makes sense. I'll get rid of that. Thanks!
Sure! How should we call that?
|
Yeah! It would be nice to have some search feature! Should I create a PR to work on search so we separate things and make the review work easier? |
Sounds good to me! |
I think |
Hi @levkk!
I cleaned the code a bit and implemented your suggestion. There are many fields in
What do you think? |
Once checks passes and unless you have objections, the code is ready to merge. 💙 |
Hey @guedes , sorry for a late reply. One last nit pick before we can merge: could you make the general stats be a vertical table? As you mentioned, there are a lot of values in there. The way one typically deals with those situations is by flipping the table, e.g.:
etc. |
Sure! I'll do it. Thanks for your feedback! |
Hi @levkk! Done! This is good for you?
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Let me know if you'd like me to merge.
Yes! Thank you! |
da76aa6
to
cea1678
Compare
Hi @levkk! I just rebased and squashed commits. Now waiting for circleci check to pass. Once test passes this PR could be merged. I'll work on other features in separated PR's. 🚀 |
cea1678
to
7bab039
Compare
Hi all.
From the @sebastianwebber's work on adding
clap
and a--help
I saw the opportunity to improve pgcat with some cli commands to eliminate some "toil".The current code is a work in progress and implements a
config
subcommand topgcat
with the goal to works as a "CRUD" for configuration, so I can edit config file without open it any editor.With this PR we can list pools with
pgcat config pools list
. See: