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

Proposal: inactivity_ms does not perform as expected/documented, let's fix that. #62

Open
grimborg opened this issue Apr 2, 2015 · 2 comments

Comments

@grimborg
Copy link
Contributor

grimborg commented Apr 2, 2015

According to the documentation, when providing a value for inactivity_ms, "If no changes happen by the timeout, Follow will signal an error." But it actually calls on_inactivity, which restarts the feed (see https://github.com/iriscouch/follow/blob/master/lib/feed.js#L624-L637).

In some cases it is not desirable to automatically retry the feed. In my case, for example, I have a large number of databases which I want to stop following once they have been inactive for a while.

Furthemore, the documented behaviour (emiting an error) seems somewhat confusing: the fact that there haven't been changes on a database should not be an error.

Instead, I would propose the following:

  • Emit a 'timeout' signal when the timeout expires.
  • Add a setting retry_after_timeout (default true), which determines whether the feed should be retried after a timeout.

This would allow users to indicate what they want Follow to do when there is a timeout, without breaking backwards compatibility.

If you are okay with this proposal, I'll implement it and send a PR.

@jcrugzz
Copy link
Member

jcrugzz commented Apr 2, 2015

@grimborg thanks for bringing this up. I made this change a while ago when I felt the erroring could be a bit too aggressive since its not a fatal case. The changes you suggested seem reasonable, I would just make it an inactive event rather than a timeout event. There is already a timeout event that has different semantics currently.

@grimborg
Copy link
Contributor Author

grimborg commented Apr 2, 2015

Thanks for the feedback! I'll emit an inactive event as you propose.

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

No branches or pull requests

2 participants