Event Manager Impementation #180

Fechado
sarah quer aplicar o merge de 0 commits de eventmanager em master
Proprietário

Barebones Event Manager Implementation.

The idea is that the cwtch app will implement an Event Manager, and each Subsystem (UI, Storage, ProtocolEngines) will have (one or more) EventQueue(s) that they can use to process the events that arrive into their subsystems.

Currently has 100% Test Coverage.

Barebones Event Manager Implementation. The idea is that the cwtch app will implement an Event Manager, and each Subsystem (UI, Storage, ProtocolEngines) will have (one or more) EventQueue(s) that they can use to process the events that arrive into their subsystems. Currently has 100% Test Coverage.
Membro
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/321
Proprietário

Queue and Manager need .Shutdown() methods, otherwise they leak threads

it prolly doesnt matter as much (I cant make as strong an argument for it, but maybe we should discus and settle on a style) but i like the style

  • lowerCase struct
  • UpperCase interface on it

It's not clear to me how Queue is to be used, it has an internal process() go routine but that just shunts messages from a channel to an internal queue. it's still left to the implementer to implement another listen thread on the queue? which doesn't have blocking options? Off the top of my head (still warming up tho) what about taking in a process function as a initialize argument that process calls on the event. Then we don't even need the queue for storage or another go routine for the client to implement

Queue and Manager need .Shutdown() methods, otherwise they leak threads it prolly doesnt matter as much (I cant make as strong an argument for it, but maybe we should discus and settle on a style) but i like the style - lowerCase struct - UpperCase interface on it It's not clear to me how Queue is to be used, it has an internal process() go routine but that just shunts messages from a channel to an internal queue. it's still left to the implementer to implement another listen thread on the queue? which doesn't have blocking options? Off the top of my head (still warming up tho) what about taking in a process function as a initialize argument that process calls on the event. Then we don't even need the queue for storage or another go routine for the client to implement
Autor
Proprietário

Will add shutdown handler.

Queue is used to prevent slow subsystems from shutting down the event bus (as go channels are blocking) - instead of blocking we automatically take the event off the event bus channel and into a private subsystem queue where we can better manage the backlog (it also allows us to identify when a given subsystem is running slow - as we can check the backlog length).

The implementer e.g. Storage has a handler which calls .Next() on Queue (which now that I think through this means that .Next() should almost certainly be blocking) - and then processes whatever event it would like.

Similar reasoning applies to not using callbacks, a slow/broken callback function would block everything - it's not too big a deal now, but when we start talking about plugins etc. I'd like to isolate each process well from the others.

I'd rather not go overboard on the struct/interface dichotomy for now - but we can discuss next time we get together.

Will add shutdown handler. Queue is used to prevent slow subsystems from shutting down the event bus (as go channels are blocking) - instead of blocking we automatically take the event off the event bus channel and into a private subsystem queue where we can better manage the backlog (it also allows us to identify when a given subsystem is running slow - as we can check the backlog length). The implementer e.g. Storage has a handler which calls .Next() on Queue (which now that I think through this means that .Next() should almost certainly be blocking) - and then processes whatever event it would like. Similar reasoning applies to not using callbacks, a slow/broken callback function would block everything - it's not too big a deal now, but when we start talking about plugins etc. I'd like to isolate each process well from the others. I'd rather not go overboard on the struct/interface dichotomy for now - but we can discuss next time we get together.
Proprietário

cool makes sense
so yeah, .Next() needs to block,

try writing a simple actual use case, and you'll quickly see what you need

cool makes sense so yeah, .Next() needs to block, try writing a simple actual use case, and you'll quickly see what you need
Membro
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/323
Membro
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/325
Membro
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/327
Autor
Proprietário

Simplified, Shutdown and Explicit Backlog Handling Added.

event.Queue is now a very light wrapper around channels, though my intuition says this likely won't be the case for long - perhaps a case of premature api optimization but once this is merged I will begin splitting out parts of peer/ into the Protocol Engine subsystem and using this code directly - which should start to poke this further.

Simplified, Shutdown and Explicit Backlog Handling Added. event.Queue is now a very light wrapper around channels, though my intuition says this likely won't be the case for long - perhaps a case of premature api optimization but once this is merged I will begin splitting out parts of peer/ into the Protocol Engine subsystem and using this code directly - which should start to poke this further.
Membro
Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/329
Proprietário

lgtm!

did you want erinn to review too before merge?

lgtm! did you want erinn to review too before merge?
Proprietário

the currently fast way to use it then is like

for event := queue.Next() {
  go process(event)
}

so we can play with that, next step may be pass in a process func to initialize and have the queue processor back and call go process(event)

like you said, we can try a few things once we have usage :)

the currently fast way to use it then is like ``` for event := queue.Next() { go process(event) } ``` so we can play with that, next step may be pass in a process func to initialize and have the queue processor back and call go process(event) like you said, we can try a few things once we have usage :)
Proprietário

add a test for publishing an event with a type that has no subscribers? other than that, lgtm (Y)

add a test for publishing an event with a type that has no subscribers? other than that, lgtm (Y)
sarah fechou este pull request 2019-01-08 00:39:50 +00:00

Pull Request Fechado

Acesse para participar desta conversação.
Sem revisor
Sem marco
Sem responsável
4 participante(s)
Notificações
Data limite
A data limite é inválida ou está fora do intervalo. Por favor, use o formato 'dd/mm/aaaa'.

Data limite não informada.

Dependências

Nenhuma dependência definida.

Referência: cwtch.im/cwtch#180
Nenhuma descrição fornecida.