On Mon, 2011-01-31 at 14:37 -0500, Eric Woods wrote:
> I've updated the driver based on David's feedback and uploaded a
> patch: https://issues.apache.org/jira/browse/DTACLOUD-15.  I'm new to
> ruby, so this feedback is really appreciated.

That looks good now, with the one exception of the @last_image business
(see below)

> Some notes:
> 2) I maintained an @last_image field to avoid an extra image lookup
> for performance reasons.  I made this more robust by checking for nil
> and performing a lookup if necessary.

Still don't like it, since it makes the default realm and HWP hard to
predict. You might get used to the fact that you'll always get HWP x
when you create an image without specifying the HWP explicitly, but then
one day your code changes, and suddenly you get HWP y.

Since we already enumerate all the HWP explicitly, just pick one as the
default (I assume COP32.1/2048/60 would be the simplest) As for realm,
just hardcode one fairly arbitrary one.

These choices at least make the defaults for create instance predictable
(and documentable ;)

> 3) The online documentation states that nothing is returned from
> reboot/stop/destroy.  We should update the documentation to indicate
> an instance is returned for each of these.

Where do you see that ? Definitely needs to be fixed.

> 4) For instances(credentials, opts={}), this method/function is being
> passed opts=nil which causes an exception, so I kept the line opts ||=
> {}

Ok .. works for me.

David


Reply via email to