Skip to content
This repository has been archived by the owner on Apr 18, 2018. It is now read-only.

after_poll #49

Open
debackel opened this issue Sep 15, 2013 · 5 comments
Open

after_poll #49

debackel opened this issue Sep 15, 2013 · 5 comments

Comments

@debackel
Copy link

Trying to test "after_poll" from a worker, I noticed that it is actually triggered 2 times at every poll cycle.

Looking at the worker.py code, “after_poll” is called from within “continue_while_connections_alive(any_activity)”, which is passed as the callback function when invoking “calling poll_connections_until_stopped”.
Within “calling poll_connections_until_stopped” , the callback function is called before starting everything else, and within the loop.

May be I misunderstood the purpose of “after_poll”, but if the purpose is to have a method called every timeout, then the right place for calling “after_poll” would be, in my opinion, within the “if time_remaining == 0.0:” section.

But simply changing the place where the “callback_fxn” (“after_poll” in the case of a call from a worker) will not do. Indeed, the callback function is used not only to trigger the after_poll method in case poll_connections_until_stopped is called from a worker, but also to call a “continue_while_jobs_pending” method in case it is called from a client.
So, we can't simply change the location of calls to “callback_fxn” within the “calling poll_connections_until_stopped” method.

I can see 2 solutions:

1/ Split the poll_connections_until_stopped method: one to be called by clients, one to be called by workers.
In the worker version of it, do not re-enter the loop when timing out. This would mean that “work” should be included in an infinite loop (as it is done in the php library). The “after_poll” would simply not exist: additional code in the infinite loop would deal with it.

2/ Add a “after_timeout(any_activity)” method in the worker and client classes, which would be called from within poll_connections_until_stopped method.

The second solution is simpler to implement, in my opinion. But we would loose the possibility to stop the polling from within after_poll (which is not an issue, in my opinion).

Does it make sense? Am I missing something? Should I propose a patch ? This is at the very heart of the code, so I'm afraid to miss a use case.

@debackel
Copy link
Author

Up. Could anyone comment on this topic?

@klange
Copy link
Contributor

klange commented Sep 24, 2013

We are in the midst of a rather extensive rewrite of some of the core internals for both workers and clients which should address this, as well as a number of other issues.

@debackel
Copy link
Author

Thank you sir.
Just for my organisation (to decide whether to wait a bit for the new version or to continue with the existing), do you have a rough estimate of the release date for this new version?

@klange
Copy link
Contributor

klange commented Sep 24, 2013

We expect at least a preliminary beta in the next few weeks.

@svisser
Copy link

svisser commented Nov 30, 2014

@klange is there more information on the rewrite and/or new version? I'm contemplating whether I should invest more time in adding unit tests and Python 3.x compatibility for #63 or whether it's better to wait.

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

No branches or pull requests

3 participants