Skip to content
This repository has been archived by the owner on Sep 8, 2024. It is now read-only.

Messagebus interface #1013

Closed
wants to merge 4 commits into from
Closed

Messagebus interface #1013

wants to merge 4 commits into from

Conversation

JarbasAI
Copy link
Contributor

I propose two new classes, BusQuery and BusResponder

BusQuery sends a message and waits for the response
BusResponder listens for messages and sends responses

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 38.593% when pulling 4576aad on JarbasAI:patch-8 into 53648b8 on MycroftAI:dev.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 38.634% when pulling e964df9 on JarbasAI:patch-8 into 53648b8 on MycroftAI:dev.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 38.628% when pulling e964df9 on JarbasAI:patch-8 into 53648b8 on MycroftAI:dev.

@penrods penrods added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Aug 22, 2017
@forslund
Copy link
Collaborator

I'm not 100% sure this is a good idea. Forcing an asynchronous system into a synchronous operation will probably not be good in the long run. We've done exactly this in a couple of places but I'm starting to feel that was a mistake.

The positive side of this approach is that it's easy to understand so in that way it's very good, I need to research what sort of performance hit we might see if this is picked up in many skills.

I would in any case look into using once instead of on when setting up the waiting task (the method will be triggered only once) and look into the possibility of using the Message.reply() method.

@JarbasAI
Copy link
Contributor Author

JarbasAI commented Aug 24, 2017

using once makes sense, will change that

the messagebus is a powerful mechanism, being able to force an asynchronous system into a synchronous operation is a very useful option to have, i have been using this in a lot of skills (which i tend to call "service_something") concurrently with success,

i agree this should not be a default, i don't like using this approach in converse method for instance, but it enables a lot more options and in the long run i think it is a useful tool

i would rather have it and not need it, than need it and not having it

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 38.628% when pulling 40dbf6b on JarbasAI:patch-8 into 53648b8 on MycroftAI:dev.

@augustnmonteiro
Copy link
Contributor

@JarbasAI @forslund I think that in this case we should use a tool that was made for that, websockets are really good if you don't need order or run sync, there are message brokers that solve this problem I would take a look at the Redis Pub/Sub https://redis.io/topics/pubsub

@penrods
Copy link
Contributor

penrods commented Feb 15, 2018

Does #1423 make this obsolete?

@JarbasAl
Copy link
Contributor

JarbasAl commented Apr 5, 2018

not exactly, #1423 solves part of what this does, but this gives more control and options

i have included this in my utils package, usage example here https://github.com/JarbasAl/mycroft_jarbas_utils/tree/master/mycroft_jarbas_utils/messagebus

@JarbasAI
Copy link
Contributor Author

JarbasAI commented Oct 2, 2018

#1822 makes this obsolete

@JarbasAI JarbasAI closed this Oct 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants