-
Notifications
You must be signed in to change notification settings - Fork 180
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
Fix broken MQTT client by implementing a reconnect "session" #1228
Conversation
0235700
to
2a7a382
Compare
Yay, that looks better:
|
...lrye-reactive-messaging-mqtt/src/main/java/io/smallrye/reactive/messaging/mqtt/MqttSink.java
Outdated
Show resolved
Hide resolved
...tt/src/main/java/io/smallrye/reactive/messaging/mqtt/session/impl/MqttClientSessionImpl.java
Outdated
Show resolved
Hide resolved
...tt/src/main/java/io/smallrye/reactive/messaging/mqtt/session/impl/MqttClientSessionImpl.java
Outdated
Show resolved
Hide resolved
...tt/src/main/java/io/smallrye/reactive/messaging/mqtt/session/impl/MqttClientSessionImpl.java
Outdated
Show resolved
Hide resolved
Ok, I have been thinking a bit about the remarks for start, subscribe, unsubscribe. Initially my thinking was that this will just record it as a "desired state", and let the state machine try to drive it towards that. However, the ClientHolder actually provides methods to monitor the state changes, and turn that into Futures/Unis. That could also be done directly be the start/subscribe/unsubscribe methods. Moving that logic there, would that be ok, from your point of view? |
a920eca
to
bda190e
Compare
Yes! |
Ok, that took a bit:
I made the changes, and tests look good now. The PR definitely needs to get squashed 😁 |
Can you squash? |
This is based on the content of PR vert-x3/vertx-mqtt#197 Fixes smallrye#1181
77b3d00
to
cdbeee1
Compare
Squashed! |
Thanks, let's see what the CI says and include it. |
Thaks a lot @ctron ! |
This is based on the content of PR vert-x3/vertx-mqtt#197
Fixes #1181