Skip to content

KAFKA-4480: Report an error in 'kafka-configs' command if the config to be removed does not exist#2218

Closed
vahidhashemian wants to merge 1 commit intoapache:trunkfrom
vahidhashemian:KAFKA-4480
Closed

KAFKA-4480: Report an error in 'kafka-configs' command if the config to be removed does not exist#2218
vahidhashemian wants to merge 1 commit intoapache:trunkfrom
vahidhashemian:KAFKA-4480

Conversation

@vahidhashemian
Copy link
Copy Markdown
Contributor

No description provided.

@guozhangwang
Copy link
Copy Markdown
Contributor

LGTM, and merged to trunk.

@asfbot
Copy link
Copy Markdown

asfbot commented Dec 31, 2016

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/409/
Test FAILed (JDK 7 and Scala 2.10).

@asfbot
Copy link
Copy Markdown

asfbot commented Dec 31, 2016

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/410/
Test FAILed (JDK 8 and Scala 2.12).

@asfbot
Copy link
Copy Markdown

asfbot commented Dec 31, 2016

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/411/
Test FAILed (JDK 8 and Scala 2.11).

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Dec 31, 2016

Wouldn't it be better to fail the whole operation? That seems safer in my opinion.

@asfgit asfgit closed this in 6f09bcb Jan 3, 2017
@guozhangwang
Copy link
Copy Markdown
Contributor

I think it is still safe to proceed the operation while filtering out the invalid config. I remember we did the same in some other console admin tools but I am not sure if it is still the case.

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jan 3, 2017

@guozhangwang, there is no way to revert an incorrect delete. If someone mistyped one config, there could be other issues in the command. It's much safer to fail the whole operation and let the user retry the whole thing. I can't think of a scenario where it's actually helpful to do a partial delete. Can you?

@guozhangwang
Copy link
Copy Markdown
Contributor

I think I agree. We could do a follow-up of this PR.

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jan 3, 2017

Sounds good. @vahidhashemian, would you mind submitting a follow-up? Thanks!

@vahidhashemian
Copy link
Copy Markdown
Contributor Author

Definitely, I'll work on submitting a follow-up PR to fail the command in case of such user errors.

soenkeliebau pushed a commit to soenkeliebau/kafka that referenced this pull request Feb 7, 2017
…to be removed does not exist

Author: Vahid Hashemian <vahidhashemian@us.ibm.com>

Reviewers: Guozhang Wang <wangguoz@gmail.com>

Closes apache#2218 from vahidhashemian/KAFKA-4480
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

Successfully merging this pull request may close these issues.

4 participants