[ISSUE-59] Adding the possibility of pass a custom prefix for Google PubSub #89
Conversation
KristianLentinoCT
force-pushed
the
feature/pubsub-subscription-prefix
branch
from
984acdf
to
8d82df0
Compare
There was a problem hiding this comment.
Thanks @KristianLentinoCT for picking this up.
I left two minor comments, you will also need to fix the pipeline due to mima exceptions (the plugin if too strict, we know...).
The error messages will guide you on how to fix that, just reach out if you need help.
| @@ -72,7 +73,7 @@ private class PubSubAdministration[F[_]]( | |||
| Subscription | |||
| .newBuilder() | |||
| .setTopic(topicName.toString()) | |||
| .setName(SubscriptionName.of(project, s"fs2-queue-$name").toString()) | |||
| .setName(SubscriptionName.of(project, s"${configs.subscriptionNamePrefix}-$name").toString()) | |||
There was a problem hiding this comment.
Minor: should we omit the -, so that one can also omit the prefix and only go with the name?
| .setName(SubscriptionName.of(project, s"${configs.subscriptionNamePrefix}-$name").toString()) | |
| .setName(SubscriptionName.of(project, s"${configs.subscriptionNamePrefix}$name").toString()) |
There was a problem hiding this comment.
I don't like that if I want to add a - in the name I should set the subscriptionNamePrefix to something like prefx-. Maybe we can be smarter and trying another approach? WDYT? Maybe we can let the PubSubConfig be an Option so that we can use only the name when the config provided is None ?
| case class PubSubConfig(subscriptionNamePrefix: String) | ||
|
|
||
| object PubSubConfig { | ||
| val default: PubSubConfig = PubSubConfig("fs2-queue") |
There was a problem hiding this comment.
Shall we just provide an empty string as a default?
I'm not sure what was the reasoning behind this choice, are there any implications on leaving this empty? @satabin maybe knows more.
| val default: PubSubConfig = PubSubConfig("fs2-queue") | |
| val default: PubSubConfig = PubSubConfig("") |
There was a problem hiding this comment.
TBH as an end user I would still prefer to have some kind of prefix instead of not having one, but I'm happy to discuss the best option with you and @satabin
Like requested on this issue I'm here to propose a way to let the user customize the prefix for Google PubSub subscriptions, while keeping the default value as it is inside the companion object of the new
PubSubConfigcase class.Let me know your thoughts!