evadne / gen_magic

Fantasia in Elixir — Binary-level file content identification, powered by libmagic
Apache License 2.0
25 stars 6 forks source link

Adding NimblePool as a supervisor child #25

Open maartenJacobs opened 1 year ago

maartenJacobs commented 1 year ago

The README states that the following should work:

children = [
  {GenMagic.Pool.NimblePool, pool_name: MyApp.GenMagicPool, pool_size: 2}
]

opts = [strategy: :one_for_one, name: MyApp.Supervisor]
Supervisor.start_link(children, opts)

But Supervisor runs child_spec/1 on its children, giving me this error:

** (Mix) Could not start application gen_magic_test: exited in: GenMagicTest.Application.start(:normal, [])
    ** (EXIT) an exception was raised:
        ** (ArgumentError) The module GenMagic.Pool.NimblePool was given as a child to a supervisor
but it does not implement child_spec/1.

I'm happy to provide a PR. I think there are at least possible 2 solutions:

%{
  id: GenMagic.Pool.NimblePool,
  start: {GenMagic.Pool.NimblePool, :start_link, [[pool_name: MyApp.GenMagicPool, pool_size: 2]]}
}
evadne commented 1 year ago

Hi @maartenJacobs the NimblePool implementation has no child spec, the Poolboy implementation has it, so the README is wrong. I’ll look into it however would suggest using Poolboy for now.

evadne commented 1 year ago

@maartenJacobs Please test feature/pool-test latest @ f81bb829395e0a0a36281f8c7e48de13186f5ced and advise, reference the relevant test files for example use cases.

devstopfix commented 1 year ago

@evadne I think you should write @maartenJacobs a cheque for £1 ;-)

Yes comparing you to Knuth - the reference https://en.wikipedia.org/wiki/Knuth_reward_check

maartenJacobs commented 1 year ago

@maartenJacobs Please test feature/pool-test latest @ f81bb82 and advise, reference the relevant test files for example use cases.

@evadne @ [f81bb82] works as expected 🙌 The pool size default works as well.

devstopfix commented 1 year ago

The pool size default works as well.

@maartenJacobs will we be using a pool size of 2 ;-)