Open Stack

On Wed, Oct 9, 2019 at 2:06 AM Frode Nordahl <frode.nordahl at canonical.com>
wrote:

> On Fri, Oct 4, 2019 at 3:46 PM Corey Bryant <corey.bryant at canonical.com>
> wrote:
>
>> Hi All,
>>
>
> Hey Corey,
>
> Great to see the charm coming along!
>
> Code is located at:
>> https://github.com/coreycb/charm-placement
>> https://github.com/coreycb/charm-interface-placement
>>
>> https://review.opendev.org/#/q/topic:charms-train-placement+(status:open+OR+status:merged)
>>
>
> 1) Since the interface is new I would love to see it based on the
> ``Endpoint`` class instead of the aging ``RelationBase`` class. Also the
> interface code needs unit tests. We have multiple examples of interface
> implementations with both in place you can get inspiration from [0].
>
> Also consider having both a ``connected`` and ``available`` state, the
> available state could be set on the first relation-changed event.  This
> increases the probability of your charm detecting a live charm in the other
> end of the relation, both states are also required to use the
> ``charms.openstack`` required relation gating code.
>
> 2) In the reactive handler you do a bespoke import of the charm class
> module just to activate the code, this is no longer necessary as there has
> been implemented a module that does automatic search and import of the
> class for you.  Please use that instead. [1]
>
>
>     import charms_openstack.bus
>     import charms_openstack.charm as charm
>
>     charms_openstack.bus.discover()
>
>
> 0:
> https://github.com/search?q=org%3Aopenstack+%22from+charms.reactive+import+Endpoint%22&type=Code
> 1:
> https://github.com/search?q=org%3Aopenstack+charms_openstack.bus&type=Code
>
> --
> Frode Nordahl
>


Hey Frode,

Thanks very much for the input. I have these up in gerrit now with the
changes you mentioned so I think we can move further reviews to gerrit:
https://review.opendev.org/#/q/topic:charms-train-placement+(status:open+OR+status:merged)

Thanks,
Corey
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-discuss/attachments/20191010/15eaecec/attachment.html>