[#3214] Decouple readiness checks from verticle deployment #3223
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
calohmn
reviewed
Apr 28, 2022
...a/src/main/java/org/eclipse/hono/client/command/kafka/KafkaBasedInternalCommandConsumer.java
Outdated
Show resolved
Hide resolved
...nts/kafka-common/src/main/java/org/eclipse/hono/client/kafka/consumer/HonoKafkaConsumer.java
Outdated
Show resolved
Hide resolved
...nts/kafka-common/src/main/java/org/eclipse/hono/client/kafka/consumer/HonoKafkaConsumer.java
Outdated
Show resolved
Hide resolved
| * | ||
| * @return A succeeded future. Note that the successful completion of the returned future does not | ||
| * mean that the consumer will be ready to receive messages from the broker. | ||
| */ | ||
| @Override | ||
| public Future<Void> start() { |
There was a problem hiding this comment.
There is currently
if (kafkaConsumer == null) {
return Future.failedFuture("not started");
in the stop() method. The failed future there marked a programming error - calling stop() before start().
Since kafkaConsumer is now null for some time after start() is completed, the stop() method should be adapted.
In case of kafkaConsumer == null it would also be best to make the KafkaClientFactory instance stop any further client creation attempts.
...nts/kafka-common/src/main/java/org/eclipse/hono/client/kafka/consumer/HonoKafkaConsumer.java
Outdated
Show resolved
Hide resolved
...nts/kafka-common/src/main/java/org/eclipse/hono/client/kafka/consumer/HonoKafkaConsumer.java
Show resolved
Hide resolved
The Kafka based clients have been adapted to provide a readiness check that is independent from the clients' startup. This is to better resemble the behavior of Hono's other components and also to not block the deployment of verticles during startup of Hono services and adapters. Fixes eclipse-hono#3214 Signed-off-by: Kai Hudalla <[email protected]>
sophokles73
force-pushed
the
decouple_readiness_check_from_startup
branch
from
05fc24f to
df18da4
Compare
|
@calohmn would you mind taking another look? |
calohmn
reviewed
May 4, 2022
...a/src/main/java/org/eclipse/hono/client/command/kafka/KafkaBasedInternalCommandConsumer.java
Outdated
Show resolved
Hide resolved
calohmn
reviewed
May 4, 2022
...a/src/main/java/org/eclipse/hono/client/command/kafka/KafkaBasedInternalCommandConsumer.java
Outdated
Show resolved
Hide resolved
...a/src/main/java/org/eclipse/hono/client/command/kafka/KafkaBasedInternalCommandConsumer.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Kai Hudalla <[email protected]>
|
@calohmn I have pushed changes |
calohmn
reviewed
May 4, 2022
...a/src/main/java/org/eclipse/hono/client/command/kafka/KafkaBasedInternalCommandConsumer.java
Outdated
Show resolved
Hide resolved
Merged
Signed-off-by: Kai Hudalla <[email protected]>
|
@calohmn I have pushed changes |
calohmn
reviewed
May 5, 2022
...a/src/main/java/org/eclipse/hono/client/command/kafka/KafkaBasedInternalCommandConsumer.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Kai Hudalla <[email protected]>
calohmn
reviewed
May 5, 2022
tests/src/test/java/org/eclipse/hono/tests/client/HonoKafkaConsumerIT.java
Show resolved
Hide resolved
Signed-off-by: Kai Hudalla <[email protected]>
|
@calohmn I have pushed a change |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Kafka based clients have been adapted to provide a readiness check
that is independent from the clients' startup. This is to better
resemble the behavior of Hono's other components and also to not block
the deployment of verticles during startup of Hono services and
adapters.
Fixes #3214