Add support for handle_continue/2 callback#300
Add support for handle_continue/2 callback#300maartenvanvliet wants to merge 6 commits intoelixir-lang:mainfrom
handle_continue/2 callback#300Conversation
* {:continue, _term} instructions can now be returned as one
would expect from gen_server.
* :hibernate is now supported on init similar to gen_server.
maartenvanvliet
force-pushed
the
handle-continue-support
branch
from
4216261 to
1a0f4a6
Compare
lib/gen_stage.ex
Outdated
| init_producer(mod, opts, state) | ||
| init_producer(mod, opts, state, nil) | ||
|
|
||
| {:producer, state, opts, continue_or_hibernate} when is_list(opts) -> |
There was a problem hiding this comment.
Not sure about the naming of continue_or_hibernate as it can also contain the timeout. Including that as well seems verbose, is there a shorter term for it?
There was a problem hiding this comment.
I'd call it additional_info.
| defp handle_gen_server_init_args(:hibernate, stage), do: {:ok, stage, :hibernate} | ||
|
|
||
| defp handle_gen_server_init_args(timeout, stage) | ||
| when (is_integer(timeout) and timeout >= 0) or timeout == :infinity, |
There was a problem hiding this comment.
Testing the timeout value here. Should this guard be placed elsewhere as well?
|
@josevalim could you have a look? |
|
@whatyouhide I know you reviewed the last revision - any chance you can take a look at this one? 🙏 |
whatyouhide
left a comment
whatyouhide
left a comment
There was a problem hiding this comment.
I’m totally down for this, @josevalim?
lib/gen_stage.ex
Outdated
| init_producer(mod, opts, state) | ||
| init_producer(mod, opts, state, nil) | ||
|
|
||
| {:producer, state, opts, continue_or_hibernate} when is_list(opts) -> |
There was a problem hiding this comment.
I'd call it additional_info.
whatyouhide
changed the title
handle_continue/2 callback
|
@josevalim @whatyouhide - Hi there! I was just looking for this feature to enable timer base scheduling in my GenStage producer. Is there a chance this could be approved soon? |
|
I need to sit down and review it. The issue is that we need to consider all possible execution scenarios for producers, consumers, and producer_consumer, and those are not trivial. So I need to be in the proper headspace and other tasks take priority at the moment. Sorry. |
6 participants
I took the liberty to address the feedback for this PR #264 as it was open for more than a year. @hazardfn hope you are okay with this?
GenServers also support a timeout in the return value
{:ok, state, timeout | :hibernate | {:continue, term}}which is currently not supported by GenStage. I've added this too.