darach / eep-js

eep.js - Embedded Event Processing
MIT License
217 stars 33 forks source link

Added an inc() call on the clock's tick function. #11

Closed panickos closed 10 years ago

panickos commented 10 years ago

Otherwise the window would not be emitted unless events were being queued into the window. The problem with that was that it was not emitting empty windows even if the application semantics required it.

panickos commented 10 years ago

Hi Darach,

Is this something you could include in eep.js release?

Cheers, Panickos

darach commented 10 years ago

Hi Panickos,

A usage scenario and examples of what you're trying to achieve would be helpful to understand what you're trying to achieve here. If events are not being queued, in a purely event driven system, then events cannot be emitted, right? That's a reasonable and deliberate constraint with the windowing system implemented here.

If you're trying to programatically 'flush' the window it might be worth adding predicate windows so that the conditions of opening, closing and emitting are programmable based on boolean expressions on the content streaming through the window. Is it possible this would suffice for your application semantic? Or, would it be possible that intermediate emission of events on some basis is what is desired?

Also, could you clarify (with a failing testcase ideally) what 'not emitting empty windows' means to you specifically? Perhaps you've found an edge case I didn't consider fully or a plain old bug / omission here. Another possibility is that I have failed to be clear on some semantic and should clarify by adding better documentation or an illustrative test case or something...

Cheers,

Darach.

panickos commented 10 years ago

Here's my use case:

I have a stream that feeds a periodic window with events. The periodic window has a size of 10 seconds. My goal is to catch the cases/windows where the stream did not enqueue any events into the window and raise an alert.

I know there are other ways of doing this but I have a kind of meta-language on top of your framework that based on some predicates, initializes window operators from eep.js. So I don't want to just hardcode this case. I just want to have a check on window emission which checks if the count is 0 the raise the alert.

Maybe you have a better suggestion?

darach commented 10 years ago

Hi Panickos,

Your use case makes sense to me. Can you add some tests to cover (document) the scenario and update the PR? Your meta language sounds interesting. Is it SQL based / like or flow based?

Cheers,

Darach.

On Mon, Mar 17, 2014 at 3:31 PM, Panickos Neophytou < notifications@github.com> wrote:

Here's my use case:

I have a stream that feeds a periodic window with events. The periodic window has a size of 10 seconds. My goal is to catch the cases/windows where the stream did not enqueue any events into the window and raise an alert.

I know there are other ways of doing this but I have a kind of meta-language on top of your framework that based on some predicates, initializes window operators from eep.js. So I don't want to just hardcode this case. I just want to have a check on window emission which checks if the count is 0 the raise the alert.

Maybe you have a better suggestion?

Reply to this email directly or view it on GitHubhttps://github.com/darach/eep-js/pull/11#issuecomment-37829216 .

panickos commented 10 years ago

Hi Darach,

Thanks for the consideration.

I started looking into the test cases. I have found some mistakes in the results which my single line commit doesn't catch (I was way optimistic about it :)).

I've noticed that you are missing some test cases of periodic windows with a specified window size (other than 0). I just wanted to see how those cases would work in order to fix my code. I also don't understand exactly how 'elapsed' would work in the tock() function once you have a sized periodic window.

I guess the first question would be if the periodic window supports anything other than 0, and if you just close windows by using the tick() function?

Cheers, Panickos

panickos commented 10 years ago

I left some comments and console.log's in there. I can remove them all after our discussion.

darach commented 10 years ago

LGTM. Please add yourself to AUTHORs and remove the console logs and i'll merge! Very well spotted indeed on the initialisation bug!

panickos commented 10 years ago

All good. Are you publishing this to npm? Cause I've been using git as the source for my packages.json.

darach commented 10 years ago

:+1: Thanks for the contribution Panickos. I'll merge and publish in npm this evening...

darach commented 10 years ago

Merged with many thanks!