Skip to content

MINOR: KIP-909 follow-ups (port validation, dead code, doc, test mock) - #23070

Merged
chia7712 merged 1 commit into
apache:trunkfrom
frankvicky:KAFKA-14648-follow-up
Aug 6, 2026
Merged

MINOR: KIP-909 follow-ups (port validation, dead code, doc, test mock)#23070
chia7712 merged 1 commit into
apache:trunkfrom
frankvicky:KAFKA-14648-follow-up

Conversation

@frankvicky

@frankvicky frankvicky commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Small review-follow-ups on top of the merged KIP-909 work (#21080,
#22897):

  • BootstrapConfiguration.enabled(...): wrap the host/port check
    with try/catch IllegalArgumentException so a malformed port (e.g.
    host:99999999999999999999 that overflows Integer.parseInt) surfaces
    as ConfigException("Invalid port in ..."), matching the pre-existing
    behaviour in ClientUtils.parseAndValidateAddresses. Without this a
    NumberFormatException would leak out of client construction.

  • ClientUtils.parseAndValidateAddresses(AbstractConfig): removed.
    This overload had no callers after the KIP-909 changes. The remaining
    two overloads ((List<String>, String) and (List<String>, ClientDnsLookup)) are still used by BrokerApiVersionsCommand and
    ConnectionStressWorker, which construct NetworkClient directly with
    BootstrapConfiguration.DISABLED and pre-resolved addresses.

  • CommonClientConfigs.BOOTSTRAP_RESOLVE_TIMEOUT_MS_DOC: reworded
    from "resolve for the bootstrap server address" to "resolve DNS for the
    bootstrap server address" for clarity.

  • KafkaProducerTest: replaced new ConsumerGroupMetadata("test-group") with
    mock(ConsumerGroupMetadata.class) +
    when(...groupId()).thenReturn("test-group") — the constructor is
    @Deprecated(forRemoval).

Reviewers: Chia-Ping Tsai chia7712@gmail.com

@github-actions github-actions Bot added producer clients triage PRs from the community small Small PRs labels Aug 3, 2026

@chia7712 chia7712 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

@@ -114,12 +114,6 @@ public static List<InetSocketAddress> parseAddresses(List<String> urls, ClientDn
return addresses;

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.

The parseAddresses method is only used by NetworkClient, but NetworkClient is already too large to add more code to ...

@chia7712
chia7712 merged commit 0720ba1 into apache:trunk Aug 6, 2026
24 checks passed
@github-actions github-actions Bot removed the triage PRs from the community label Aug 7, 2026
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.

2 participants