Skip to content

MINOR: Suppress header-less partitioner deprecation - #23134

Open
UladzislauBlok wants to merge 2 commits into
apache:trunkfrom
UladzislauBlok:bloku/partitioner-suppression
Open

MINOR: Suppress header-less partitioner deprecation#23134
UladzislauBlok wants to merge 2 commits into
apache:trunkfrom
UladzislauBlok:bloku/partitioner-suppression

Conversation

@UladzislauBlok

@UladzislauBlok UladzislauBlok commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

as a title
We could think about promoting some set of standard test partitioners into common util, but would be overkill IMO

Reviewers: Matthias J. Sax matthias@confluent.io

@UladzislauBlok UladzislauBlok changed the title Suppress header-less partitioner deprecation MINOR: Suppress header-less partitioner deprecation Aug 11, 2026
@github-actions github-actions Bot added triage PRs from the community streams tests Test fixes (including flaky tests) labels Aug 11, 2026
@mjsax mjsax added ci-approved and removed triage PRs from the community labels Aug 11, 2026

@mjsax mjsax 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 cleanup. Made a pass.

}
}

private static class FixedPartitionPartitioner implements StreamPartitioner<Integer, Object> {

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.

Suggested change
private static class FixedPartitionPartitioner implements StreamPartitioner<Integer, Object> {
private static class FixedPartitionPartitioner implements StreamPartitioner<String, String> {

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 removed code also uses <String, String>

private StreamPartitioner<String, String> constantPartitioner(final Integer partition)

partitionerInvocation.incrementAndGet();
return Optional.of(Collections.singleton(partition));
.withStreamPartitioner(new StreamPartitioner<>() {
@SuppressWarnings({"removal"})

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.

Suggested change
@SuppressWarnings({"removal"})
@SuppressWarnings("removal")

return config;
}

private static class FixedPartitionPartitioner implements StreamPartitioner<Integer, Object> {

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.

Seems we are duplicating code here (3x in this PR), plus already existing FixedPartitionPartitioner in StreamsMetadataStateTest).

Might be worth to move to test-fixtures module and have it only a single time?

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.

I mentioned it. Just wandering if 4 occurrences is enough to extract it
Don't think we expect any more uses of this partitoner + after 5.0 we're going to remove it (and we could back to lambda syntax)

@mjsax mjsax Aug 12, 2026

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.

Guess should be fine either way. (Don't think there is a "count threshold"; well, for production code there is, and it's 2 -- we should not have any duplication there. But for test-only code, we can handle it more loosley)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-approved streams tests Test fixes (including flaky tests)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants