allthingssecurity commented on code in PR #28317:
URL: https://github.com/apache/camel/pull/28317#discussion_r4355258040
##########
components/camel-dapr/src/main/java/org/apache/camel/component/dapr/consumer/DaprConfigurationConsumer.java:
##########
@@ -74,10 +77,25 @@ protected void doStart() throws Exception {
client = getEndpoint().getClient();
SubscribeConfigurationRequest configRequest = new
SubscribeConfigurationRequest(configStore, configKeys);
- Flux<SubscribeConfigurationResponse> responses =
client.subscribeConfiguration(configRequest);
+ // the stream ends when the call to the sidecar fails or is closed
(for example when the sidecar restarts), and
+ // the Dapr client does not open it again: subscribe again (after a
delay) for as long as the consumer runs, else
+ // the consumer stays started and receives no further configuration
changes
+ Flux<SubscribeConfigurationResponse> responses = Flux.defer(() ->
client.subscribeConfiguration(configRequest))
+ .doOnComplete(() -> LOG.warn(
+ "The subscription to configuration changes of store {}
ended. Subscribing again in {} ms",
+ configStore, RESUBSCRIBE_DELAY.toMillis()))
+ .repeatWhen(completed ->
completed.delayElements(RESUBSCRIBE_DELAY))
Review Comment:
Intentional: a completed stream is not an error (the sidecar ended it
cleanly), so the consumer subscribes again after 1 s, the same delay the first
retry uses; the doubling to 30 s is for failures, where hammering a sidecar
that is down does harm. A sidecar that keeps completing streams right after
subscribing would be polled once a second, which is cheap and recovers fastest
once it is up. Happy to share the backoff between both paths if a maintainer
prefers that.
_Claude Code on behalf of allthingssecurity_
##########
components/camel-dapr/src/main/java/org/apache/camel/component/dapr/consumer/DaprConfigurationConsumer.java:
##########
@@ -88,7 +106,7 @@ protected void doStart() throws Exception {
AsyncCallback cb = defaultConsumerCallback(exchange, true);
Review Comment:
Agreed, it is a safety net: `Retry.backoff(Long.MAX_VALUE, ...)` keeps the
consumer subscribing for the life of the route, and the error callback only
matters if that ever changes. I'll leave the code as it is unless a maintainer
wants a comment there.
_Claude Code on behalf of allthingssecurity_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
