-
Notifications
You must be signed in to change notification settings - Fork 489
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 Key/Value interface #830
Conversation
ec2dc5c
to
d4858b5
Compare
6397136
to
30315e1
Compare
cc91b01
to
fb5a707
Compare
Update documentation of CLI producer Print status for each record produced, unless -q
f06ac4a
to
d4ffb1c
Compare
@@ -34,6 +48,10 @@ pub struct ConsumeLogOpt { | |||
#[structopt(short = "d", long)] | |||
pub disable_continuous: bool, | |||
|
|||
/// Print records in "[key] value" format, with "[null]" for no key | |||
#[structopt(short, long)] | |||
pub key_value: bool, |
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.
suggest kv
for shorthand
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 has the structopt(short)
, which means we can use --key-value
or -k
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.
actually, this flag is confusing. it sounds like it is about consuming. it should be more like: --print-kv-header
. Also printing key should be default so in this case, it should be -disable-print-kv-header
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.
Originally I had made the consumer print keys by default, but @ajhunyady said he preferred otherwise. I don't have a preference one way or the other. Let me know what you guys decide.
73fc088
to
6acc582
Compare
680770f
to
038916b
Compare
@sehz I think all of the changes have been added. I'm ready to merge this if you are 👍 |
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.
Great work!
* Producer CLI with --lines Update documentation of CLI producer Print status for each record produced, unless -q * Add initial Key/Value support to CLI producer and consumer * Apply cargo clippy * Add support for JSON Key/Value with jsonpath * Update producer printout * Update consumer key/value printout formatting * Update interactive printout for producer and consumer * Remove --partition from producer * Add --key-value to consumer CLI * Update help menu for consumer CLI * Condense producer CLI code * Remove --json-path from producer CLI * Review: update tracing and error printout in consumer * Remove multi-file support for producer * Remove 'Reading one record per line from stdin' prompt * Default producer behavior to always be line-by-line * Update producer help * Fix double-printing produced lines * Fixed printout for producer interactive mode
Adds a key/value interface for the producer and consumer CLI. Should not be merged before #828.
The first draft of the documentation for these CLI changes is here: infinyon/fluvio-website#58