You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "dajac (via GitHub)" <gi...@apache.org> on 2024/12/04 08:50:35 UTC
[PR] KAFKA-17598; Command line validation tool for RE2J regex [kafka]
dajac opened a new pull request, #18031:
URL: https://github.com/apache/kafka/pull/18031
This patch introduces the `--validate-regex` argument to the `kafka-consumer-group` command line tool as defined in KIP-848. The new argument allows the verification of RE2 regular expressions.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17598; Command line validation tool for RE2J regex [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm commented on code in PR #18031:
URL: https://github.com/apache/kafka/pull/18031#discussion_r1869586621
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommand.java:
##########
@@ -130,6 +155,15 @@ else if (opts.options.has(opts.resetOffsetsOpt)) {
}
}
+ static void validateRegex(String regex) {
+ try {
+ Pattern.compile(regex);
+ System.out.printf("The regular expressions `%s` is valid.%n", regex);
Review Comment:
```suggestion
System.out.printf("The regular expression `%s` is valid.%n", regex);
```
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommandOptions.java:
##########
@@ -210,7 +216,9 @@ private ConsumerGroupCommandOptions(String[] args) {
void checkArgs() {
CommandLineUtils.maybePrintHelpOrVersion(this, "This tool helps to list all consumer groups, describe a consumer group, delete consumer group info, or reset consumer group offsets.");
- CommandLineUtils.checkRequiredArgs(parser, options, bootstrapServerOpt);
+ if (!options.has(validateRegexOpt)) {
+ CommandLineUtils.checkRequiredArgs(parser, options, bootstrapServerOpt);
+ }
Review Comment:
agree, but should we then update the bootstrap doc that states it's required?
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommand.java:
##########
@@ -130,6 +155,15 @@ else if (opts.options.has(opts.resetOffsetsOpt)) {
}
}
+ static void validateRegex(String regex) {
+ try {
+ Pattern.compile(regex);
+ System.out.printf("The regular expressions `%s` is valid.%n", regex);
+ } catch (PatternSyntaxException ex) {
+ System.out.printf("The regular expressions `%s` is invalid: %s.%n", regex, ex.getDescription());
Review Comment:
```suggestion
System.out.printf("The regular expression `%s` is invalid: %s.%n", regex, ex.getDescription());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17598; Command line validation tool for RE2J regex [kafka]
Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on PR #18031:
URL: https://github.com/apache/kafka/pull/18031#issuecomment-2517488705
A little checkstyle to fix :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17598; Command line validation tool for RE2J regex [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #18031:
URL: https://github.com/apache/kafka/pull/18031#discussion_r1869564566
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommand.java:
##########
@@ -95,7 +97,14 @@ public static void main(String[] args) {
ConsumerGroupCommandOptions opts = ConsumerGroupCommandOptions.fromArgs(args);
try {
// should have exactly one action
- long actions = Stream.of(opts.listOpt, opts.describeOpt, opts.deleteOpt, opts.resetOffsetsOpt, opts.deleteOffsetsOpt).filter(opts.options::has).count();
+ long actions = Stream.of(
+ opts.listOpt,
+ opts.describeOpt,
+ opts.deleteOpt,
+ opts.resetOffsetsOpt,
+ opts.deleteOffsetsOpt,
+ opts.validateRegexOpt
+ ).filter(opts.options::has).count();
if (actions != 1)
CommandLineUtils.printUsageAndExit(opts.parser, "Command must include exactly one action: --list, --describe, --delete, --reset-offsets, --delete-offsets");
Review Comment:
Nice catch! I fixed it. Result:
```
Command must include exactly one action: --list, --describe, --delete, --reset-offsets, --delete-offsets, --validate-regex
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17598; Command line validation tool for RE2J regex [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #18031:
URL: https://github.com/apache/kafka/pull/18031#discussion_r1869622426
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommandOptions.java:
##########
@@ -210,7 +216,9 @@ private ConsumerGroupCommandOptions(String[] args) {
void checkArgs() {
CommandLineUtils.maybePrintHelpOrVersion(this, "This tool helps to list all consumer groups, describe a consumer group, delete consumer group info, or reset consumer group offsets.");
- CommandLineUtils.checkRequiredArgs(parser, options, bootstrapServerOpt);
+ if (!options.has(validateRegexOpt)) {
+ CommandLineUtils.checkRequiredArgs(parser, options, bootstrapServerOpt);
+ }
Review Comment:
yeah, i considered doing this but i did not like it. how would you state it? "required except for --validate-regex"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17598; Command line validation tool for RE2J regex [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac merged PR #18031:
URL: https://github.com/apache/kafka/pull/18031
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17598; Command line validation tool for RE2J regex [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #18031:
URL: https://github.com/apache/kafka/pull/18031#discussion_r1869643993
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommandOptions.java:
##########
@@ -210,7 +216,9 @@ private ConsumerGroupCommandOptions(String[] args) {
void checkArgs() {
CommandLineUtils.maybePrintHelpOrVersion(this, "This tool helps to list all consumer groups, describe a consumer group, delete consumer group info, or reset consumer group offsets.");
- CommandLineUtils.checkRequiredArgs(parser, options, bootstrapServerOpt);
+ if (!options.has(validateRegexOpt)) {
+ CommandLineUtils.checkRequiredArgs(parser, options, bootstrapServerOpt);
+ }
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17598; Command line validation tool for RE2J regex [kafka]
Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #18031:
URL: https://github.com/apache/kafka/pull/18031#discussion_r1869185562
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommand.java:
##########
@@ -95,7 +97,14 @@ public static void main(String[] args) {
ConsumerGroupCommandOptions opts = ConsumerGroupCommandOptions.fromArgs(args);
try {
// should have exactly one action
- long actions = Stream.of(opts.listOpt, opts.describeOpt, opts.deleteOpt, opts.resetOffsetsOpt, opts.deleteOffsetsOpt).filter(opts.options::has).count();
+ long actions = Stream.of(
+ opts.listOpt,
+ opts.describeOpt,
+ opts.deleteOpt,
+ opts.resetOffsetsOpt,
+ opts.deleteOffsetsOpt,
+ opts.validateRegexOpt
+ ).filter(opts.options::has).count();
if (actions != 1)
CommandLineUtils.printUsageAndExit(opts.parser, "Command must include exactly one action: --list, --describe, --delete, --reset-offsets, --delete-offsets");
Review Comment:
Probably ought to add `validate-regex` into the usage message.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17598; Command line validation tool for RE2J regex [kafka]
Posted by "lianetm (via GitHub)" <gi...@apache.org>.
lianetm commented on code in PR #18031:
URL: https://github.com/apache/kafka/pull/18031#discussion_r1869638746
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommandOptions.java:
##########
@@ -210,7 +216,9 @@ private ConsumerGroupCommandOptions(String[] args) {
void checkArgs() {
CommandLineUtils.maybePrintHelpOrVersion(this, "This tool helps to list all consumer groups, describe a consumer group, delete consumer group info, or reset consumer group offsets.");
- CommandLineUtils.checkRequiredArgs(parser, options, bootstrapServerOpt);
+ if (!options.has(validateRegexOpt)) {
+ CommandLineUtils.checkRequiredArgs(parser, options, bootstrapServerOpt);
+ }
Review Comment:
it currently is "REQUIRED: The server(s) to connect to.". Maybe flipping it to "The server(s) to connect to. REQUIRED for all options except for --validate-regex"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-17598; Command line validation tool for RE2J regex [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #18031:
URL: https://github.com/apache/kafka/pull/18031#discussion_r1869642539
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ConsumerGroupCommandOptions.java:
##########
@@ -210,7 +216,9 @@ private ConsumerGroupCommandOptions(String[] args) {
void checkArgs() {
CommandLineUtils.maybePrintHelpOrVersion(this, "This tool helps to list all consumer groups, describe a consumer group, delete consumer group info, or reset consumer group offsets.");
- CommandLineUtils.checkRequiredArgs(parser, options, bootstrapServerOpt);
+ if (!options.has(validateRegexOpt)) {
+ CommandLineUtils.checkRequiredArgs(parser, options, bootstrapServerOpt);
+ }
Review Comment:
works for me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org