Re: Layer-Supervisor

2016-07-21 Thread Charles Butler
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


Re: Layer-Supervisor

2016-07-21 Thread Adam Stokes
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


Re: Layer-Supervisor

2016-07-21 Thread James Beedy
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


Re: Layer-Supervisor

2016-07-21 Thread Adam Stokes
One thing (which I haven't done yet in some of my layers) is to place your
supervisorlib in `lib/charms/layer`. A nice first start though!

On Thu, Jul 21, 2016 at 4:39 AM, Stuart Bishop 
wrote:

> On 21 July 2016 at 10:40, James Beedy  wrote:
>
> > I'll take all the input/feedback/criticism I can get on this - thanks!
>
> Under Multi Application Support, the example references myapp1_ctxt
> and myapp2_ctxt variables but they are both undefined and there is no
> mention on what they are supposed to contain.
>
> From a charming perspective, I'm interested in if I should be using
> Supervisor or systemd to control my applications. If you want uptake,
> a simple Python API for generating simple templates is preferable to
> expecting us to read the Supervisor documentation and learn its
> configuration syntax ;)
>
> I think you want a wheelhouse.txt listing supervisor in your layer, so
> the dependency gets pulled in at 'charm build' time, or the Ubuntu
> package listed in layer.yaml under options->basic->packages. I don't
> see anywhere in the layer that is installing the dependency.
>
> --
> Stuart Bishop 
>
> --
> 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


Re: Layer-Supervisor

2016-07-21 Thread Stuart Bishop
On 21 July 2016 at 10:40, James Beedy  wrote:

> I'll take all the input/feedback/criticism I can get on this - thanks!

Under Multi Application Support, the example references myapp1_ctxt
and myapp2_ctxt variables but they are both undefined and there is no
mention on what they are supposed to contain.

>From a charming perspective, I'm interested in if I should be using
Supervisor or systemd to control my applications. If you want uptake,
a simple Python API for generating simple templates is preferable to
expecting us to read the Supervisor documentation and learn its
configuration syntax ;)

I think you want a wheelhouse.txt listing supervisor in your layer, so
the dependency gets pulled in at 'charm build' time, or the Ubuntu
package listed in layer.yaml under options->basic->packages. I don't
see anywhere in the layer that is installing the dependency.

-- 
Stuart Bishop 

-- 
Juju mailing list
Juju@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju


Layer-Supervisor

2016-07-20 Thread James Beedy
Funny it had to get to this point... I'm seemingly tired of copying/writing
the same supervisor app config template rendering function everywhere, so I
took a stab at layer-supervisor
<https://github.com/jamesbeedy/layer-supervisor>. Layer-Supervisor follows
in the footsteps of layer-node, and layer-ruby (thanks Adam). It
could probably use some polish, but I think its pretty straight forward and
reasonably solid for the most part.

I'll take all the input/feedback/criticism I can get on this - thanks!
-- 
Juju mailing list
Juju@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju