Skip to content
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

cli's help messages are too abstract, e.g. nothing toxic specific #141

Open
Dieterbe opened this issue Oct 24, 2016 · 1 comment
Open

cli's help messages are too abstract, e.g. nothing toxic specific #141

Dieterbe opened this issue Oct 24, 2016 · 1 comment

Comments

@Dieterbe
Copy link

Dieterbe commented Oct 24, 2016

out of #139 came an interesting point.
per @xthexder:

The cli is designed to be independant of what toxics are defined, so that if a new toxic is added, the cli and api will still work with older versions

interesting choice. this explains another gripe i had as a newcomer to this project: toxiproxy-cli only has help messages for highlevel operations (e.g. create and remove a toxic) but no help messages about which toxics you can add, what their attributes are, etc. It would be nice to be able to have toxiproxy-cli toxic help add list the available toxics and toxiproxy-cli toxic help add latency for example show details about how to add a latency toxic. With the current strategy of keeping the cli generic, this can't be done.
You could technically have the cli query the server for the available toxics, their attributes, which are required vs optional etc and then feed that back to the user, but I would argue that's getting a bit complicated and probably makes it quite hard to dynamically build optimized CLI experiences (see below)

If somebody upgrades their server to support new toxics or new toxic attributes, then it seems very reasonable that they also update the cli as well. given proper packaging / install script /.. this is no extra effort at all.

this way the cli can "truly" support the toxics. and not only could it display more useful help messages, it could also provide a CLI experience that's more optimized. e.g. instead of:
toxiproxy-cli toxic add [command options] <proxyName> with options like --type value, -t value and --attribute value, -a value

you could do:
toxiproxy-cli toxic add [command options] latency <proxyName> with options like --latency and --jitter

this CLI approach seems more user friendly IMHO, I would argue it's a good thing that the cli is optimized for the terminology of the toxics etc instead of trying to be more of a generic json encoder for toxic attributes. If people want that, they can use curl ;)

anyway, please consider this as the way i intend it: constructive feedback.
this project is pretty sweet and I hope to contribute as much as I can (first with some thoughts and ideas, and later hopefully with code as well)

@xthexder
Copy link
Contributor

There's a couple reasons why things were done the way they are, though that's not to say they couldn't be improved.

The first is that having all clients toxic independent (and this this case, the cli is a client), means that when a toxic is added or changed, no further development time is needed to update all the clients, they will work as-is.
This is important for 3rd party clients especially, though for things like the cli and toxiproxy-ruby it's just a matter of ease of development.

The second reason is that toxiproxy does have support for 3rd party toxics, which currently only require using github.com/Shopify/toxiproxy and an include to build a custom server binary.
Requiring a cli build as well makes that process much more complicated, unless the generic format is also kept, but having a separate usage pattern for these toxics seems counter productive to me.

Some sort of way to list available toxics on the server seems like a reasonable addition, though I'm not sure of the best way to handle toxic fields in that scenario.
The experience for new toxiproxy users is definitely something that could be improved, it will take some discussion to figure out what changes to make though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants