-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Make agents propagate events immediately #167
Conversation
@@ -6,7 +6,8 @@ | |||
class Event < ActiveRecord::Base | |||
include JSONSerializedField | |||
|
|||
attr_accessible :lat, :lng, :payload, :user_id, :user, :expires_at | |||
attr_accessor :propagate_immediately |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you need this accessor anymore, because now it's on the Agent.
Nice work! Have you ran this for a while and seen that the propagation works the way we expect? |
So far, yes. Though I don't have much more than a toy setup, currently. |
I'll try running it on my setup for a few days. |
Whoops, specs are failing. |
Hey @qedi-r, it looks like there is an error in the build. |
Oops. Missed one last line of the unused propagate_immediately in events... |
Nice! I will test this for a few days with my Huginn setup, then merge it. |
@@ -62,6 +62,7 @@ | |||
@agent.source_ids), | |||
{}, { :multiple => true, :size => 5, :class => 'span4 select2' }) %> | |||
<span class='cannot-receive-events text-info'>This type of Agent cannot receive events.</span> | |||
<span class="propagate-immediately"><br><%= f.check_box :propagate_immediately %> <%= f.label "Propagate immediately", :class => 'control-label' %></span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this inside of the same control-group
as sources for any particular reason? Could you move it to a separate one so that it sits on one line?
I think this would work:
<div class="control-group">
<% f.label :propagate_immediately, :class => 'control-label' do %>
<%= f.check_box :propagate_immediately %>
<% end %>
</div>
You may need a tiny but of CSS so that it's vertically centered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, it's in the same group because it only makes sense if an Agent has sources, right?
Could you put the checkbox and label on the same line, perhaps by nesting them as in my code above?
I don't think if separating it out into it's own control-group is necessary, so that isn't part of this, otherwise, the label makes more sense now. |
It's been working well for me, so I'm going to merge it. Nice work! |
Make agents propagate events immediately
Make agents propagate events immediately
Might be nice to add a tooltip to the web interface explaining what this option does. I'd be up for adding the tooltip, but I'm not totally sure what it should say. |
Good idea. That'd be great if you could add one! Maybe something like "Selecting this option causes any listening Agents to receive the event immediately without waiting for a scheduled propagation. Be sure to avoid loops." |
So just to be clear, "propagate immediately" is set on the creating agent (push semantics), correct? Just some confusion because I believe #157 seemed to suggest otherwise. In any case, I think it may be useful to also have a similar setting to be set on the receiving agent (pull semantics), so when a scheduled even occurs, it can set off a chain-reaction of events in order to fetch the data it needs. For example, I have a "Get Weather" agent that fetches conditions at 5am, and then a "Morning Digest" agent at 6am. By the time I get the weather, it's already an hour out-of-date. This would rectify that issue. Thoughts? |
Hey @elsurudo, actually this is pull semantics, and I agree it's a bit confusing. We have a def possibly_propagate
#immediately schedule agents that want immediate updates
propagate_ids = agent.receivers.where(:propagate_immediately => true).pluck(:id)
Agent.receive!(:only_receivers => propagate_ids) unless propagate_ids.empty?
end This is actually looking at the creating Agent's receivers and pushing the event to any that have requested immediate propagation. |
I see. I would still call that push semantics, with the only difference being that the receivers and not the senders request the immediate propagation. How would you handle my use case — to fetch information at the immediate time that a morning digest is being sent out, to ensure it is as current as possible? Maybe there is still a use-case for true pull semantics, to allow you to set the schedule at the end of your pipeline, and then the upstream agents would propagate? Admittedly I am very new to Huginn, so I may be missing something, an perhaps there are easier ways to accomplish what I want...
|
I usually schedule the required agents slightly earlier and allow the events to flow into the dependent agents, but you have a valid use case. There is also the SchedulerAgent for specific control. |
This should be the first step to resolving #157