Add Support for AzureServiceBusSource#1418
Conversation
odacremolbap
left a comment
There was a problem hiding this comment.
Almost there,
I left a question regarding the queue case and the status condition set.
| limitations under the License. | ||
| */ | ||
|
|
||
| package servicebus |
There was a problem hiding this comment.
I understand this will remain and the other copies will be removed.
Does not feel great to have copies around, let's remember to remove the one at servicebustopics
Alternatively we could remove it now and redirect the previous code to this client, but that is not needed now on my side.
There was a problem hiding this comment.
Yes, that's the plan, as soon as we delete the old azure servicebus topic and queue I'll remove this with the rest of the old code.
| // In the case when TopicID is present, it is necessary to ensure the subscription. | ||
| // when dealing with QueueID, there's no need. | ||
| if o.Spec.TopicID != nil { |
There was a problem hiding this comment.
The queue case will leave the status set to Unknown ...
If that is the case we should either try to make the status conditions dynamic (not sure if that is something that we have done at any other component), or set the subscribed condition to true with a reason that says NOTNEEDED.
There was a problem hiding this comment.
It should be fixed now
This PR add support for Azure ServiceBus Source, allowing to use in the same source topics and queues