Add unsubscribe method so client can unsubscribe from topics#90
Merged
danyi1212 merged 5 commits intopermitio:masterfrom Jun 24, 2025
Merged
Add unsubscribe method so client can unsubscribe from topics#90danyi1212 merged 5 commits intopermitio:masterfrom
danyi1212 merged 5 commits intopermitio:masterfrom
Conversation
3 tasks
Contributor
|
Hi @nneskildsf this looks good all in all, please add tests for the new flow. |
c7be264 to
16727b3
Compare
Contributor
Author
Thank you for the encouragement. In order to implement a test, I had to implement the following in
My implementation maintains support for not awaiting calls to the Looking forward to feedback. Br Eskild |
16727b3 to
12d19ca
Compare
Contributor
Author
|
Ping @orweis. Have a great day! |
Contributor
|
@danyi1212 will review |
Contributor
Author
|
Ping @danyi1212. Have a great day! |
danyi1212
approved these changes
Jun 24, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi
My application is long running and the client will subscribe and unsubscribe to different topics during its lifetime. To this end this PR adds an
unsubscribemethod to go along with thesubscribemethod.I figure that other users might also benefit from this addition, thus this PR.
The method returns
Trueand logs a warning if a request is made to unsubscribe a topic which does not exist. This is in line with the behaviour offastapi_websocket_pubsub.event_notifier.EventNotifier.unsubscribewhich also returnsTrueif a subscription is removed with a non-existent subscriber id.I see that the current test cases are quite readable and appear like best-practice examples of how to use the library. I don't want to disturb that style with tedious tests. Do let me know if you think test cases are required.
Br Eskild