Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KCA: Option to sanitize topic name for the connectors that cannot handle pulsar topic names #14475

Merged
merged 1 commit into from
Feb 28, 2022

Conversation

dlg99
Copy link
Contributor

@dlg99 dlg99 commented Feb 25, 2022

Motivation

Some kafka connectors do not sanitize topic names (or incompletely do) to match what downstream system supports.
It works in kafka in most cases, assuming appropriately named topics. This does not work well with Kafka Connect Adaptor because URI part is getting there.

Modifications

  • Flag to sanitize the topic name (disabled by default) and corresponding functionality.
  • test

Verifying this change

  • Make sure that the change passes the CI checks.

  • added unit test

  • verified with specific connector that didn't work without this change

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (yes / no)
  • The public API: (yes / no)
  • The schema: (yes / no / don't know)
  • The default values of configurations: (yes / no)
  • The wire protocol: (yes / no)
  • The rest endpoints: (yes / no)
  • The admin cli options: (yes / no)
  • Anything that affects deployment: (yes / no / don't know)

Documentation

Check the box below or label this PR directly (if you have committer privilege).

Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    Config parameter documented in FieldDoc.

@github-actions github-actions bot added the doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. label Feb 25, 2022
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Very nice and useful
I left some suggestions PTAL

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

+1

Copy link
Member

@michaeljmarshall michaeljmarshall left a comment

Choose a reason for hiding this comment

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

LGTM

@michaeljmarshall michaeljmarshall merged commit fd9e639 into apache:master Feb 28, 2022
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Mar 1, 2022
…dle pulsar topic names (apache#14475)

Some kafka connectors do not sanitize topic names (or incompletely do) to match what downstream system supports.
It works in kafka in most cases, assuming appropriately named topics. This does not work well with Kafka Connect Adaptor because URI part is getting there.

* Flag to sanitize the topic name (disabled by default) and corresponding functionality.
* test

- [ ] Make sure that the change passes the CI checks.

- added unit test
- verified with specific connector that didn't work without this change

*If `yes` was chosen, please highlight the changes*

  - Dependencies (does it add or upgrade a dependency): (yes / no)
  - The public API: (yes / no)
  - The schema: (yes / no / don't know)
  - The default values of configurations: (yes / no)
  - The wire protocol: (yes / no)
  - The rest endpoints: (yes / no)
  - The admin cli options: (yes / no)
  - Anything that affects deployment: (yes / no / don't know)

Check the box below or label this PR directly (if you have committer privilege).

Need to update docs?

- [ ] `doc-required`

  (If you need help on updating docs, create a doc issue)

- [ ] `no-need-doc`

  (Please explain why)

- [x] `doc`

  Config parameter documented in FieldDoc.

(cherry picked from commit fd9e639)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Mar 1, 2022
…dle pulsar topic names (apache#14475)

Some kafka connectors do not sanitize topic names (or incompletely do) to match what downstream system supports.
It works in kafka in most cases, assuming appropriately named topics. This does not work well with Kafka Connect Adaptor because URI part is getting there.

* Flag to sanitize the topic name (disabled by default) and corresponding functionality.
* test

- [ ] Make sure that the change passes the CI checks.

- added unit test
- verified with specific connector that didn't work without this change

*If `yes` was chosen, please highlight the changes*

  - Dependencies (does it add or upgrade a dependency): (yes / no)
  - The public API: (yes / no)
  - The schema: (yes / no / don't know)
  - The default values of configurations: (yes / no)
  - The wire protocol: (yes / no)
  - The rest endpoints: (yes / no)
  - The admin cli options: (yes / no)
  - Anything that affects deployment: (yes / no / don't know)

Check the box below or label this PR directly (if you have committer privilege).

Need to update docs?

- [ ] `doc-required`

  (If you need help on updating docs, create a doc issue)

- [ ] `no-need-doc`

  (Please explain why)

- [x] `doc`

  Config parameter documented in FieldDoc.

(cherry picked from commit fd9e639)
eolivelli pushed a commit that referenced this pull request Mar 22, 2022
…dle pulsar topic names (#14475)

### Motivation

Some kafka connectors do not sanitize topic names (or incompletely do) to match what downstream system supports.
It works in kafka in most cases, assuming appropriately named topics. This does not work well with Kafka Connect Adaptor because URI part is getting there.

### Modifications

* Flag to sanitize the topic name (disabled by default) and corresponding functionality.
* test

### Verifying this change

- [ ] Make sure that the change passes the CI checks.

- added unit test
- verified with specific connector that didn't work without this change

### Does this pull request potentially affect one of the following parts:

*If `yes` was chosen, please highlight the changes*

  - Dependencies (does it add or upgrade a dependency): (yes / no)
  - The public API: (yes / no)
  - The schema: (yes / no / don't know)
  - The default values of configurations: (yes / no)
  - The wire protocol: (yes / no)
  - The rest endpoints: (yes / no)
  - The admin cli options: (yes / no)
  - Anything that affects deployment: (yes / no / don't know)

### Documentation

Check the box below or label this PR directly (if you have committer privilege).

Need to update docs?

- [ ] `doc-required`

  (If you need help on updating docs, create a doc issue)

- [ ] `no-need-doc`

  (Please explain why)

- [x] `doc`

  Config parameter documented in FieldDoc.

(cherry picked from commit fd9e639)
Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022
…dle pulsar topic names (apache#14475)

### Motivation

Some kafka connectors do not sanitize topic names (or incompletely do) to match what downstream system supports.
It works in kafka in most cases, assuming appropriately named topics. This does not work well with Kafka Connect Adaptor because URI part is getting there.

### Modifications

* Flag to sanitize the topic name (disabled by default) and corresponding functionality.
* test

### Verifying this change

- [ ] Make sure that the change passes the CI checks.

- added unit test
- verified with specific connector that didn't work without this change

### Does this pull request potentially affect one of the following parts:

*If `yes` was chosen, please highlight the changes*

  - Dependencies (does it add or upgrade a dependency): (yes / no)
  - The public API: (yes / no)
  - The schema: (yes / no / don't know)
  - The default values of configurations: (yes / no)
  - The wire protocol: (yes / no)
  - The rest endpoints: (yes / no)
  - The admin cli options: (yes / no)
  - Anything that affects deployment: (yes / no / don't know)

### Documentation

Check the box below or label this PR directly (if you have committer privilege).

Need to update docs? 

- [ ] `doc-required` 
  
  (If you need help on updating docs, create a doc issue)
  
- [ ] `no-need-doc` 
  
  (Please explain why)
  
- [x] `doc` 
  
  Config parameter documented in FieldDoc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Your PR contains doc changes, no matter whether the changes are in markdown or code files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants