Skip to content

Conversation

nneskildsf
Copy link
Contributor

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 unsubscribe method to go along with the subscribe method.
I figure that other users might also benefit from this addition, thus this PR.

The method returns True and logs a warning if a request is made to unsubscribe a topic which does not exist. This is in line with the behaviour of fastapi_websocket_pubsub.event_notifier.EventNotifier.unsubscribe which also returns True if 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

@orweis
Copy link
Contributor

orweis commented May 20, 2025

Hi @nneskildsf this looks good all in all, please add tests for the new flow.

@nneskildsf nneskildsf force-pushed the add-unsubscribe-method branch from c7be264 to 16727b3 Compare May 20, 2025 20:17
@nneskildsf
Copy link
Contributor Author

Hi @nneskildsf this looks good all in all, please add tests for the new flow.

Thank you for the encouragement.

In order to implement a test, I had to implement the following in pub_sub_client.py:

  • Post connection subscription
  • Unsubscribe

My implementation maintains support for not awaiting calls to the subscribe method if it is made before a connection is established. It will still work if the call is awaited though. If the connection is established, then calls to subscribe and unsubscribe must be awaited and warnings will be presented by the event loop if they aren't.

Looking forward to feedback.

Br Eskild

@nneskildsf nneskildsf force-pushed the add-unsubscribe-method branch from 16727b3 to 12d19ca Compare May 20, 2025 20:24
@nneskildsf
Copy link
Contributor Author

Ping @orweis. Have a great day!

@orweis
Copy link
Contributor

orweis commented Jun 2, 2025

@danyi1212 will review

@nneskildsf
Copy link
Contributor Author

Ping @danyi1212. Have a great day!

@danyi1212 danyi1212 merged commit b7f593c into permitio:master Jun 24, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants