Code review comment for lp:~franciscosouza/pyjuju/juju-vpc

Revision history for this message
Kapil Thangavelu (hazmat) wrote :

no worries about using lbox, i don't mind having a look on occasion, i
understood from the merge proposal that it wasn't intended for merge. i've
been going through the txaws branches as well, which look good but i'll
need to rope in one more reviewer for them i think. i'd like to get some
additional functionality there for txaws to be able to create vpc, with a
goal that juju can create the vpc itself. cheers.

On Thu, Nov 22, 2012 at 8:58 AM, <email address hidden> wrote:

>
> https://codereview.appspot.**com/6850044/diff/7017/juju/**
> providers/ec2/securitygroup.py<https://codereview.appspot.com/6850044/diff/7017/juju/providers/ec2/securitygroup.py>
> File juju/providers/ec2/**securitygroup.py (right):
>
> https://codereview.appspot.**com/6850044/diff/7017/juju/**
> providers/ec2/securitygroup.**py#newcode11<https://codereview.appspot.com/6850044/diff/7017/juju/providers/ec2/securitygroup.py#newcode11>
> juju/providers/ec2/**securitygroup.py:11: for group in d.result:
> On 2012/11/22 13:44:07, hazmat wrote:
>
>> This is very suspect, deferred results should not be access directly,
>>
> this needs
>
>> a callback registered or inlineCallbacks syntax.
>>
>
> Thanks, I will fix this.
>
> I will also stop updating this CL using lbox, so you guys stop receiving
> notifications. There is more than VPC in this CL now (proxy,
> pre-installed juju, etc.), and we will probably keep using this fork. I
> hope that we will be able to send smaller patchs for merging in the
> (near) future.
>
> https://codereview.appspot.**com/6850044/<https://codereview.appspot.com/6850044/>
>

« Back to merge proposal