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

ChangesFollower.init/1 doesn't comply with GenServer spec #3

Open
dch opened this issue Sep 18, 2018 · 1 comment
Open

ChangesFollower.init/1 doesn't comply with GenServer spec #3

dch opened this issue Sep 18, 2018 · 1 comment

Comments

@dch
Copy link

dch commented Sep 18, 2018

It should be possible to drop an instance of ChangesFollower directly into a Supervision tree, so it can act as a normally managed Worker, but it returns a different set of tuples than expected, so this fails.

As ChangesFollower registers as a behaviour, it must comply with the GenServer spec:

 @callback init(args :: term()) ::
            {:ok, state}
            | {:ok, state, timeout() | :hibernate | {:continue, term()}}
            | :ignore
            | {:stop, reason :: any()}
          when state: any()

However its return spec is different, returning:

  @callback init(args :: term) ::
    {:ok, db :: ICouch.DB.t, state} |
    {:ok, db :: ICouch.DB.t, opts :: [option], state} |
    {:ok, db :: ICouch.DB.t, opts :: [option], state, timeout | :hibernate} |
    :ignore |
{:stop, reason :: any} when state: term

There's presumably a reason for doing it this way, but the result is that an additional GenServer-compliant behaviour needs to be wrapped around this before it can be used correctly in a standard OTP supervision tree.

Would you take a breaking change PR for ChangesFollower that deals with the extraneous items db, opts simply by removing them entirely from the returned tuple? It seems that any process spawning a ChangesFollower will already have the DB and options it passes into the worker.

I'm still working on a wrapper but how do you handle this at meetnow? It seems better to achieve this directly in ICouch.

@dch
Copy link
Author

dch commented Dec 19, 2018

any comments?

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

1 participant