Skip to content

[Functions] Remove unused class which didn't get reverted - #11707

Merged
eolivelli merged 1 commit into
apache:masterfrom
lhotari:lh-remove-unused-class-PulsarCluster
Aug 24, 2021
Merged

[Functions] Remove unused class which didn't get reverted#11707
eolivelli merged 1 commit into
apache:masterfrom
lhotari:lh-remove-unused-class-PulsarCluster

Conversation

@lhotari

@lhotari lhotari commented Aug 18, 2021

Copy link
Copy Markdown
Member

Motivation

Modifications

remove pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/PulsarCluster.java which is unused

- this should have been part of apache#10843 which reverted apache#8434 changes
@lhotari lhotari added the type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use label Aug 18, 2021
@lhotari lhotari added this to the 2.9.0 milestone Aug 18, 2021
@lhotari lhotari self-assigned this Aug 18, 2021
Comment on lines -69 to -75
if (producerSpec.getBatchBuilder() != null) {
if (producerSpec.getBatchBuilder().equals("KEY_BASED")) {
this.producerBuilder.batcherBuilder(BatcherBuilder.KEY_BASED);
} else {
this.producerBuilder.batcherBuilder(BatcherBuilder.DEFAULT);
}
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's pretty odd that this class happened to contain the logic related to #8523 that was missing from ContextImpl. That problem is fixed by #11706.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I found the explanation: the required code went missing as part of reverting #8434 changes in #10843 . The KEY_BASED logic had been added to the PulsarCluster class after #8434 had been merged.

@lhotari
lhotari requested a review from wolfstudy August 18, 2021 19:49
@Anonymitaet

Copy link
Copy Markdown
Member

@lhotari double check: no need to update docs?

@lhotari lhotari added the doc-not-needed Your PR changes do not impact docs label Aug 19, 2021
@lhotari

lhotari commented Aug 19, 2021

Copy link
Copy Markdown
Member Author

@lhotari double check: no need to update docs?

@Anonymitaet no need. I added the label.

@eolivelli eolivelli left a comment

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.

LGTM

@eolivelli
eolivelli merged commit 9558c54 into apache:master Aug 24, 2021
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants