-
Notifications
You must be signed in to change notification settings - Fork 669
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
MF-1630 - Replace old subscriptions with a new one instead of throwing an error #1633
Conversation
…ects Signed-off-by: aryan <aryangodara03@gmail.com>
@AryanGodara Can you please check why CI is failling? |
Yes, I'm working on it. |
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #1633 +/- ##
==========================================
- Coverage 69.16% 68.96% -0.20%
==========================================
Files 138 138
Lines 11231 11350 +119
==========================================
+ Hits 7768 7828 +60
- Misses 2771 2828 +57
- Partials 692 694 +2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AryanGodara This needs to be done for all the supported message brokers (MQTT and RabbitMQ) so that all the implementations match as much as possible.
Signed-off-by: aryan <aryangodara03@gmail.com>
… replace-subscription
6ceb2e1
to
48875da
Compare
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you can see, code in different implementations is repeated - mostly the code that holds the subscriptions. You can consider how we can improve this and prevent code repetition.
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
6e0c243
to
ff7f146
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This starts to look very good to me now. I can approve this one practically, I'll just wait for a few comments from @dborovcanin and we are ready to merge, I think.
… replace-subscription
Signed-off-by: aryan <aryangodara03@gmail.com>
8a76d95
to
c72e5fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once the remark in MQTT PubSub is resolved, we're ready.
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
Signed-off-by: aryan <aryangodara03@gmail.com>
e3629a6
to
d484937
Compare
Signed-off-by: aryan <aryangodara03@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: AryanGodara aryangodara03@gmail.com
What does this do?
Replaced old subscriptions instead the
pubsub.Subscribe()
, instead of returning analreadySubscribed
errorWhich issue(s) does this PR fix/relate to?
Resolves #1630
List any changes that modify/break current functionality
N/A
Have you included tests for your changes?
No
Did you document any new/modified functionality?
No
Notes
N/A