Fixes and Enhancements to group reset functionality#2546
Conversation
MikeEdgar
force-pushed
the
group-reset-cleanup
branch
3 times, most recently
from
ac04a9d to
1675541
Compare
MikeEdgar
force-pushed
the
group-reset-cleanup
branch
2 times, most recently
from
6905d35 to
a2a6671
Compare
MikeEdgar
force-pushed
the
group-reset-cleanup
branch
from
a2a6671 to
d527742
Compare
jankalinic
left a comment
jankalinic
left a comment
There was a problem hiding this comment.
LGTM, can't run test's against this right now, I'll fix the login page shortly, if you want to wait for the reset offset ST.
Assisted-by: IBM Bob Signed-off-by: Michael Edgar <medgar@redhat.com>
MikeEdgar
force-pushed
the
group-reset-cleanup
branch
from
d527742 to
4071551
Compare
alexcreasy
left a comment
alexcreasy
left a comment
There was a problem hiding this comment.
1 comment I wasn't 100% sure on.
I tested this on our cloud deployment and it works using dry runs. No complaints with the code. UX seems straightforward for doing this.
LGTM.
| state: ResetOffsetFormState, | ||
| topicName?: string | ||
| ): string { | ||
| let command = `$ kafka-consumer-groups --bootstrap-server \${BOOTSTRAP_SERVER} --group '${groupId}' `; |
There was a problem hiding this comment.
It looks deliberate that the bootstrap server is a literal string rather than a templatevar, but I just wanted to check this is the case?
It's not actually changed in this PR but it just stuck out at me as something to check!
There was a problem hiding this comment.
That's right - we keep that as a variable because there may be multiple bootstrap addresses for them to choose from.
|
3 participants
Refine the group offset reset functionality in new client-side React UI.
Assisted-by: IBM Bob