-
Notifications
You must be signed in to change notification settings - Fork 55
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
single process chat listening #127
Comments
@songgao mentioned tracking conversationId internally in the library to manage this. That seems good. the one issue is I don't know how the library knows a possible chat's |
Good point on non-existent conversations. I guess I took it for granted when working on the GUI code. As part of brainstorming, how about something like this as an exported helper function that generates a string which can be used later to compare conversations/channels:
Then we'll take Kind of off topic but related, if I remember correctly public conversations is not a supported thing anymore, so looks like Edit: add missing not in last paragraph |
I wouldn't overcomplicate it, defining a standard way the usernames are sorted should be enough, the actual comparsions will be 2-3 lines of code, so it's not that bad. If we really want to do this, then an alternative would be bundling the conversation ID inside of the channel object - that should solve the unknown channel issue at the cost of more data going through the data stream. I'm all for the subscription stuff, if this gets to CORE I'd love to work on this. For now I think it'd be worth it to modify the existing code to use a single api-listen process, it won't that terribly difficult. |
I think I'd even postpone the single process change for now. It's effectively a performance improvement, which I agree with, but there are more missing features we should probably get through first. |
ideally:
the bot logic doesn't need to do any analysis, upgrading, or comparing channel objects to know whether a watch request matches an incoming message, even though a user may have referred to the same chat in a different way
there's only one chat api listen call made, so we don't need like 100 of them to listen to 100 channels
the bot library doesn't need to receive and process messages it doesn't care about
internally, we should work with CORE to make chat api listen take subscription additions and cancellations via stdin. something where we pass it a watchId we invent and a channel to watch, and it starts including any matches in its output, along with the watchId. This achieves all our goals while keeping code simple on this side and for people who want to make their own libs in other languages.
cc @pzduniak @songgao . not necessarily something to work on right now, but it seems like the right answer.
The text was updated successfully, but these errors were encountered: