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

"start" and "stop" event for cubism.context #19

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rhuss
Copy link

@rhuss rhuss commented Jun 26, 2012

These events are useful for sources which needs synchronization e.g. for starting/stopping and internal scheduler for pulling periodically data (like Jolokia does).

@rhuss
Copy link
Author

rhuss commented Apr 17, 2013

Is there any chance to get this (simple) pull request applied ?

@mbostock
Copy link
Collaborator

I’m not sure it’s sufficiently useful (meaning: essential) to add to the API. If it’s your code that’s calling context.start and context.stop, you can coordinate that method call with other operations that your code depends on.

It seems a little funny to add this responsibility to the context, when in general, classes aren’t responsible for reporting events when you call methods on them. On the other hand, since the context already broadcasts events for its state, I suppose you could argue that whether the context is running or not is reasonable to add to the API.

If your metric wants to prefetch data on a regular schedule, rather than pulling data on demand as the metric API is intended, you can use the existing metric API to control your scheduled pulls implicitly through timeouts. All you have to do is pause the periodic pulls if your metric’s request function hasn’t been called in a while.

@rhuss
Copy link
Author

rhuss commented Apr 18, 2013

Maybe explaining my use case clarifies a bit, why I want to hook into the context's lifecycle.

I uses cubism for graphing live data obtained from Jolokia which is a JMX-HTTP bridge. Therefore I implemented an new data source which periodically fetches this live data and stores it in an array. You can see it in action on this demo page, the documentation can be found here.

My point is, that I need a second scheduler for live measuring the data on a regular basis (with fixed ticks), independent from the cubism scheduler. So, it is not about prefetching data from a backend store, which already has a timeseries for the charting data available.

So I think it is a good idea would be to couple the lifecycles of both schedulers, where this pull request provides an easy way for.

Of course the alternative is to manage both lifecycles externally:

   var context = cubism.context();
   var jolokia = context.jolokia("http://jolokia.org/jolokia");

   // Create some metrics ....

   jolokia.start();
   context.start();

This is the way how it is done now, with this patch one could save the jolokia.start() call. This would be nice also from an API perspective in order to hide implementation details of this data source.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants