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

KAFKA-17529: Remove blacklist from MM2 #17202

Merged
merged 16 commits into from
Oct 7, 2024
Merged

Conversation

chiacyu
Copy link
Contributor

@chiacyu chiacyu commented Sep 15, 2024

As titled, check KAFKA-17529 for details.
Please take a look, thanks!

Committer Checklist (excluded from commit message)

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

@chia7712
Copy link
Contributor

@chiacyu thanks for your patch. Could you please cleanup the test of

Also, please update upgrade.html

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@chiacyu thanks for your patch. please check testConfigBackwardsCompatibility also

@@ -51,6 +51,18 @@ <h5><a id="upgrade_400_notable" href="#upgrade_400_notable">Notable changes in 4
The <code>--whitelist</code> option was removed from the <code>kafka-console-consumer</code> command line tool.
Please use <code>--include</code> instead.
</li>
<li>
The <code>--whitelist</code> option was removed from the <code>kafka-console-consumer</code> command line tool.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this unrelated change

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@chiacyu thanks for updated patch. Could you please remove groups.blacklist also?

@@ -69,8 +69,8 @@ public void testConfigPropertyMatching() {
@Test
public void testConfigBackwardsCompatibility() {
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need this test because the blacklist-related configs are removed, right?

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@chiacyu thanks for your updates

Please use <code>config.properties.exclude</code> instead.
</li>
<li>
The <code>topics.blacklist</code> was removed from the <code>kafka.connect.mirror.MirrorSourceConfig</code>
Copy link
Contributor

Choose a reason for hiding this comment

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

org.apache.kafka.connect.mirror.MirrorSourceConfig

Please use <code>topics.exclude</code> instead.
</li>
<li>
The <code>groups.blacklist</code> was removed from the <code>kafka.connect.mirror.MirrorSourceConfig</code>
Copy link
Contributor

Choose a reason for hiding this comment

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

org.apache.kafka.connect.mirror.MirrorSourceConfig

@@ -51,6 +51,18 @@ <h5><a id="upgrade_400_notable" href="#upgrade_400_notable">Notable changes in 4
The <code>--whitelist</code> option was removed from the <code>kafka-console-consumer</code> command line tool.
Please use <code>--include</code> instead.
</li>
<li>
The <code>config.properties.blacklist</code> was removed from the <code>kafka.connect.mirror.MirrorSourceConfig</code>
Copy link
Contributor

Choose a reason for hiding this comment

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

org.apache.kafka.connect.mirror.MirrorSourceConfig

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@chiacyu thanks for updates

null,
Importance.HIGH,
"Deprecated. Use " + GROUPS_EXCLUDE_CONFIG + " instead.");
GROUPS_EXCLUDE_DOC);

GroupFilterConfig(Map<String, ?> props) {
super(DEF, ConfigUtils.translateDeprecatedConfigs(props, new String[][]{
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need to call translateDeprecatedConfigs now, since the deprecated configs are removed

@@ -89,7 +83,7 @@ static class ConfigPropertyFilterConfig extends AbstractConfig {

ConfigPropertyFilterConfig(Map<String, ?> props) {
super(DEF, ConfigUtils.translateDeprecatedConfigs(props, new String[][]{
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need to call translateDeprecatedConfigs now, since the deprecated configs are removed

@@ -81,7 +80,7 @@ public class MirrorCheckpointConfig extends MirrorConnectorConfig {

public MirrorCheckpointConfig(Map<String, String> props) {
super(CONNECTOR_CONFIG_DEF, ConfigUtils.translateDeprecatedConfigs(props, new String[][]{
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need to call translateDeprecatedConfigs now, since the deprecated configs are removed

null,
Importance.HIGH,
"Deprecated. Use " + TOPICS_EXCLUDE_CONFIG + " instead.");
TOPICS_EXCLUDE_DOC);

TopicFilterConfig(Map<String, ?> props) {
super(DEF, ConfigUtils.translateDeprecatedConfigs(props, new String[][]{
Copy link
Contributor

Choose a reason for hiding this comment

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

it does not need translateDeprecatedConfigs, right?

@@ -66,6 +75,8 @@ <h5><a id="upgrade_400_notable" href="#upgrade_400_notable">Notable changes in 4
<li>The <code>--whitelist</code> option was removed from the <code>kafka-console-consumer</code> command line tool.
Please use <code>--include</code> instead.
</li>
<li>The <code>--delete-config</code> option in the <code>kafka-topics</code> command line tool has been deprecated.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the unnecessary changes

@@ -98,8 +96,8 @@ public class MirrorSourceConfig extends MirrorConnectorConfig {

public MirrorSourceConfig(Map<String, String> props) {
super(CONNECTOR_CONFIG_DEF, ConfigUtils.translateDeprecatedConfigs(props, new String[][]{
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need translateDeprecatedConfigs, right?

@chia7712 chia7712 merged commit 9e809be into apache:trunk Oct 7, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants