Thank you all, for your feedback!
@alex tracking
@adam 10-4
@charles |
That all makes sense -the reason I wanted it in lib, was because I was
leaving it open to the user of the layer to configure supervisor for their
app(s).
Would it be preferable to have the user specify a list of appnames as
a config param, and iterate over them?
On Thu, Jul 21, 2016 at 2:59 PM, Charles Butler <
charles.but...@canonical.com> wrote:
> Greetings James,
>
> +1 to Adams suggestion.
>
> A question about the readme examples:
>
> There's a given example where you're getting a Workers() object but it
> isn't imported, or declared. Where did it come from?
>
> @when('myapp1.supervisor.available',
> 'myapp2.supervisor.available')def run_workers():
> w = Workers()
> w.start_tasks()
>
>
> Emittersare really states, I would probably rename that header for
> clarity.
>
> Looking into the code, it looks like a great start. Some minor nits though:
>
>
> https://github.com/jamesbeedy/layer-supervisor/blob/master/lib/charms/layer/supervisorlib.py#L43
>
> There's some host modifications going on in here I wouldn't expect to find
> here. Really I would expect to see this bit handled in the reactive layer.
>
> Maybe place it around:
>
> https://github.com/jamesbeedy/layer-supervisor/blob/master/reactive/supervisor.py#L11
>
> And try to keep host modifications to the reactive module, and keep the
> library tidy for interfacing with supervisor and doing the redundant things
> for the user when those come about like scaling workers, providing sensible
> defaults for common values.
>
> I see quite a bit in here http://supervisord.org/configuration.html so
> you're definitely on the right path :)
>
>
> All in all its a great start, I look forward to seeing where this goes,
> and would like to thank you for your continued contributions to the charm
> and layer ecosystem.
>
> All the best,
>
> Charles
>
>
>
> On Thu, Jul 21, 2016 at 1:25 PM Adam Stokes <adam.sto...@canonical.com>
> wrote:
>
>> looks good, one small nit pick, maybe change supervisorlib.py to just
>> supervisor.py. that seems to be the current naming scheme of things
>>
>> On Thu, Jul 21, 2016 at 1:42 PM, James Beedy <jamesbe...@gmail.com>
>> wrote:
>>
>>> A big thanks to those of you who gave feedback! I have made some
>>> revisions based on the suggestions I received from a few of you (thank
>>> you!), and would like to get a second round of feedback now the
>>> modifications have been made -> [layer-supervisor](
>>> https://github.com/jamesbeedy/layer-supervisor).
>>>
>>> Thanks all!
>>>
>>> --
>>> Juju mailing list
>>> Juju@lists.ubuntu.com
>>> Modify settings or unsubscribe at:
>>> https://lists.ubuntu.com/mailman/listinfo/juju
>>>
>>>
>> --
>> Juju mailing list
>> Juju@lists.ubuntu.com
>> Modify settings or unsubscribe at:
>> https://lists.ubuntu.com/mailman/listinfo/juju
>>
> --
> Juju Charmer
> Canonical Group Ltd.
> Ubuntu - Linux for human beings | www.ubuntu.com
> Juju - The fastest way to model your service | www.jujucharms.com
>
--
Juju mailing list
Juju@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/juju
