[#3549] Improved getOrCreateTopic/Subscription flow for Pub/Sub #3550
Conversation
...ubsub-common/src/main/java/org/eclipse/hono/client/pubsub/PubSubBasedAdminClientManager.java
Outdated
Show resolved
Hide resolved
...ubsub-common/src/main/java/org/eclipse/hono/client/pubsub/PubSubBasedAdminClientManager.java
Outdated
Show resolved
Hide resolved
mattkaem
force-pushed
the
improve-pubsub-getOrCreateTopic-and-subscription
branch
from
431cf59 to
f3c0f2b
Compare
...ubsub-common/src/main/java/org/eclipse/hono/client/pubsub/PubSubBasedAdminClientManager.java
Outdated
Show resolved
Hide resolved
...ubsub-common/src/main/java/org/eclipse/hono/client/pubsub/PubSubBasedAdminClientManager.java
Outdated
Show resolved
Hide resolved
mattkaem
force-pushed
the
improve-pubsub-getOrCreateTopic-and-subscription
branch
from
8ebca7c to
0ab4653
Compare
| * A factory to create PubSubBasedAdminClientManagerImpl instances. | ||
| */ | ||
| @ApplicationScoped | ||
| public class PubSubBasedAdminClientManagerFactoryImpl implements PubSubBasedAdminClientManagerFactory { |
There was a problem hiding this comment.
what's the purpose of having this factory? It doesn't seem to do anything but instantiate the real PubSubBasedAdminClientManager, which could be done directly by the users of this factory, or am I mistaken?
There was a problem hiding this comment.
No, you are right. It doesn't do anything here. I just need it for the Device Communication API in the hono-extras repository, so I just moved it over there.
| } | ||
|
|
||
| private Future<String> getSubscription(final SubscriptionName subscription, final SubscriptionAdminClient client) { | ||
| return vertx.executeBlocking(promise -> promise.tryComplete(client.getSubscription(subscription).getName())); |
There was a problem hiding this comment.
Using promise.tryComplete here means that the promise should be completed with the value if it has not yet been completed. I suspect, however, that you intended to complete the promise only in case of the client.getSubscription invocation having succeeded, right?
If so, then you need to wrap the client.getSubscription invocation in a try/catch block and either invoke promise.complete or promise.fail based on the outcome ...
There was a problem hiding this comment.
You are right. Somehow it seemed to work like I had it before, but I definitely like your suggestion more, so I implemented it.
…ub/Sub messaging infrastructure and changed the subscription expiration to never Signed-off-by: Matthias Kaemmer <[email protected]>
mattkaem
force-pushed
the
improve-pubsub-getOrCreateTopic-and-subscription
branch
from
b5c0e27 to
0561fa0
Compare
Improved getOrCreateTopic/Subscription flow for Pub/Sub messaging infrastructure and changed the subscription expiration to never by adding the default ExpirationPolicy instance to the subscription