-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Added ability to specify producer config for functions and sources #7721
Conversation
@@ -108,6 +108,14 @@ public ContextImpl(InstanceConfig config, Logger logger, PulsarClient client, | |||
|
|||
this.producerBuilder = (ProducerBuilderImpl<?>) client.newProducer().blockIfQueueFull(true).enableBatching(true) | |||
.batchingMaxPublishDelay(1, TimeUnit.MILLISECONDS); | |||
if (config.getFunctionDetails().getSink().getProducerSpec() != null) { | |||
if (config.getFunctionDetails().getSink().getProducerSpec().getMaxPendingMessages() != 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the variable is Integer
, shouldn't we check for != null
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are checking for null in getProducerSpec.
The getFunctionDetails is a protobuf and the variable that it returns is an integer. So null checking is not needed
@srkukarni you also need to set to producer configs for the producers in PulsarSink |
@jerrypeng done |
import org.apache.pulsar.common.functions.FunctionConfig; | ||
import org.apache.pulsar.common.functions.Resources; | ||
import org.apache.pulsar.common.functions.WindowConfig; | ||
import org.apache.pulsar.common.functions.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit lets not use wild cards. Makes debugging harder
…pache#7721) * Added ability to specify producer config for functions and sources * Fixed test * Fix test * Add generated function proto * Add header * Address comments Co-authored-by: Sanjeev Kulkarni <sanjeevk@splunk.com>
…pache#7721) * Added ability to specify producer config for functions and sources * Fixed test * Fix test * Add generated function proto * Add header * Address comments Co-authored-by: Sanjeev Kulkarni <sanjeevk@splunk.com>
…pache#7721) * Added ability to specify producer config for functions and sources * Fixed test * Fix test * Add generated function proto * Add header * Address comments Co-authored-by: Sanjeev Kulkarni <sanjeevk@splunk.com>
…pache#7721) * Added ability to specify producer config for functions and sources * Fixed test * Fix test * Add generated function proto * Add header * Address comments Co-authored-by: Sanjeev Kulkarni <sanjeevk@splunk.com>
…pache#7721) * Added ability to specify producer config for functions and sources * Fixed test * Fix test * Add generated function proto * Add header * Address comments Co-authored-by: Sanjeev Kulkarni <sanjeevk@splunk.com>
(If this PR fixes a github issue, please add
Fixes #<xyz>
.)Fixes #
(or if this PR is one task of a github issue, please add
Master Issue: #<xyz>
to link to the master issue.)Master Issue: #
Motivation
Currently when the framework creates producers for either the output topic or side topics, it is not possible to configure those producers. This pr adds that capability
Modifications
Describe the modifications you've done.
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If
yes
was chosen, please highlight the changesDocumentation