Code review comment for lp:~jimbaker/pyjuju/expose-provider-ec2

Revision history for this message
Jim Baker (jimbaker) wrote :

On Tue, Jul 26, 2011 at 9:19 AM, William Reade
<email address hidden>wrote:

> Review: Needs Fixing
> * Tiny, irrelevant note: rather than directly creating an
> EC2ProviderMachine, there's now a machine_from_instance function in
> ec2.machine which is a touch more convenient.
>

Now using

>
> * Significant issue: I don't like having both 'machine' and 'machine_id'
> parameters: if there isn't currently a way to determine machine_id given a
> machine, I think it would be worth adding one and using it, to avoid
> cluttering up the Provider interface.
>

Changed such that ProviderMachine now takes an optional machine_id so this
can be passed around. In general, this field needs to be assigned after the
fact (EC2 doesn't know these machine IDs from our topology), and this is
what is done in the provisioning agent. But it does provide a central place
for this information instead of having to pass around two related pieces.

« Back to merge proposal