Skip to content

KAFKA-8974: trim whitespaces in topic#7442

Merged
rhauch merged 3 commits intoapache:trunkfrom
mageshn:KAFKA-8974
Oct 4, 2019
Merged

KAFKA-8974: trim whitespaces in topic#7442
rhauch merged 3 commits intoapache:trunkfrom
mageshn:KAFKA-8974

Conversation

@mageshn
Copy link
Copy Markdown
Contributor

@mageshn mageshn commented Oct 3, 2019

This is a minor change that will improve the UX when the topic list includes whitespace in the topics config for a SinkConnector

Committer Checklist (excluded from commit message)

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


if (SinkConnectorConfig.hasTopicsConfig(taskConfig)) {
String[] topics = taskConfig.get(SinkTask.TOPICS_CONFIG).split(",");
Arrays.parallelSetAll(topics, i->topics[i].trim());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we really need to process this in parallel? seems like setAll() should be simple and clean enough.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah, that's a good point. I have modified the code per your suggestion.

Copy link
Copy Markdown
Contributor

@wicknicks wicknicks left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Copy Markdown
Contributor

@rhauch rhauch left a comment

Choose a reason for hiding this comment

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

Thanks, @mageshn. LGTM.

Valid characters for Kafka topics are the ASCII alphanumerics, '.', '_', and '-', so trimming the topic names before assignment makes sense.

@rhauch rhauch self-requested a review October 4, 2019 17:25
Copy link
Copy Markdown
Contributor

@rhauch rhauch left a comment

Choose a reason for hiding this comment

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

@mageshn, the checkstyle run has failed:

'->' is not followed by whitespace. | 290
'->' is not preceded with whitespace. | 290

…/WorkerSinkTask.java

Co-Authored-By: Randall Hauch <rhauch@gmail.com>
@rhauch rhauch merged commit 586c587 into apache:trunk Oct 4, 2019
rhauch pushed a commit that referenced this pull request Oct 4, 2019
…#7442)

Trim whitespaces in topic names specified in sink connector configs before subscribing to the consumer. Topic names don't allow whitespace characters, so trimming only will eliminate potential problems and will not place additional limits on topics specified in sink connectors.

Author: Magesh Nandakumar <magesh.n.kumar@gmail.com>
Reviewers: Arjun Satish <arjun@confluent.io>, Randall Hauch <rhauch@gmail.com>
rhauch pushed a commit that referenced this pull request Oct 4, 2019
…#7442)

Trim whitespaces in topic names specified in sink connector configs before subscribing to the consumer. Topic names don't allow whitespace characters, so trimming only will eliminate potential problems and will not place additional limits on topics specified in sink connectors.

Author: Magesh Nandakumar <magesh.n.kumar@gmail.com>
Reviewers: Arjun Satish <arjun@confluent.io>, Randall Hauch <rhauch@gmail.com>
rhauch pushed a commit that referenced this pull request Oct 4, 2019
…#7442)

Trim whitespaces in topic names specified in sink connector configs before subscribing to the consumer. Topic names don't allow whitespace characters, so trimming only will eliminate potential problems and will not place additional limits on topics specified in sink connectors.

Author: Magesh Nandakumar <magesh.n.kumar@gmail.com>
Reviewers: Arjun Satish <arjun@confluent.io>, Randall Hauch <rhauch@gmail.com>
rhauch pushed a commit that referenced this pull request Oct 4, 2019
…#7442)

Trim whitespaces in topic names specified in sink connector configs before subscribing to the consumer. Topic names don't allow whitespace characters, so trimming only will eliminate potential problems and will not place additional limits on topics specified in sink connectors.

Author: Magesh Nandakumar <magesh.n.kumar@gmail.com>
Reviewers: Arjun Satish <arjun@confluent.io>, Randall Hauch <rhauch@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants