Skip to content
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

Fix: don't throw warning for unsubscribe results #195

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lmagyar
Copy link
Contributor

@lmagyar lmagyar commented Jun 17, 2024

Without waiting for the reply, it always created a warning: Received result for unknown message with ID: x

it always created a warning: Received result for unknown message with ID: x
@marcelveldt
Copy link
Member

Continuation of the other reply. Maybe we should not care and just log this to verbose. In nay case its not worth it to await the unsub reply unless I'm missing some usecase here.

Are you perhaps interested in doing sub+unsub during runtime ?
The usecase for me is just setup all subs at startup/connect and only unsub at shutdown/disconnect.

Maybe all this takes is a separation of the subscribe and unsubscribe, make them 2 individual functions and leave it up the caller to handle.

@lmagyar
Copy link
Contributor Author

lmagyar commented Jun 18, 2024

Yes, sub+unsub during runtime. :)

Hmmm, and how will the caller identify the subscription if it is a separete method? Currently the id is captured in the function, this is sooo elegant.

The only thing that bothered me, was the warning, if the warning is only a debug/verbose message, I'm fine with it.

@lmagyar lmagyar changed the title Enhancement: wait for the unsubscribe reply Fix: don't throw warning for unsubscribe results Jun 18, 2024
@marcelveldt
Copy link
Member

I was thinking of giving back the subscription ID so you can store that and decide for yourself if you want to unsubscribe.
But lets adjust it the other way and keep the ubsub function in the subscribe result but leave it as a "set and forget" task.
So ignoring the log is the goal.

@marcelveldt marcelveldt added the enhancement New feature or request label Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants