Skip to content

KAFKA-6973: TopicCommand should verify topic-level config#5106

Merged
ijuma merged 2 commits into
apache:trunkfrom
huxihx:KAFKA-6973
Jun 1, 2018
Merged

KAFKA-6973: TopicCommand should verify topic-level config#5106
ijuma merged 2 commits into
apache:trunkfrom
huxihx:KAFKA-6973

Conversation

@huxihx

@huxihx huxihx commented May 31, 2018

Copy link
Copy Markdown
Contributor

KAFKA-6973: https://issues.apache.org/jira/browse/KAFKA-6973

Specifying an invalid config other than CreateTime or LogAppendTime will cause broker restarting to fail.

More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

…mestamp.type`.

Specifying an invalid config other than `CreateTime` or `LogAppendTime` will cause broker restarting to fail.
@huxihx

huxihx commented May 31, 2018

Copy link
Copy Markdown
Contributor Author

@ijuma Please review this minor patch. Thanks.

@omkreddy

Copy link
Copy Markdown
Contributor

LGTM.

@huxihx

huxihx commented Jun 1, 2018

Copy link
Copy Markdown
Contributor Author

@omkreddy thanks for responding, so can it be merged to the trunk?

@ijuma ijuma left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, just a minor comment.

TestUtils.createBrokersInZk(zkClient, brokers)

val topic = "test"
val numPartitions = 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need these variables since they're just used once.

@ijuma ijuma left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ijuma ijuma merged commit 0120e88 into apache:trunk Jun 1, 2018
ying-zheng pushed a commit to ying-zheng/kafka that referenced this pull request Jul 6, 2018
Specifying an invalid config (i.e. something other than `CreateTime` or
`LogAppendTime`) via `TopicCommand` would previously cause the
broker to fail on start-up.

Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>, Ismael Juma <ismael@juma.me.uk>
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.

3 participants