Thanks, David. See responses below. Unfortunately I'm not able to provide a test account for IBM's cloud, so we'll have to "fake it" for test cases. Is writing/running tests documented anywhere?
Thanks, - Eric W. On Feb 1, 2011, at 6:26 PM, David Lutterkort wrote: > 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 ;) The issue is not all hardware profiles are valid for all images. For each image, a list of valid HWP configurations are provided. This mandates that I check the corresponding image to determine a valid "default." I noticed that images() is called when a create request is issued, so I stored the image there rather than performing an additional lookup later. I recognize that this is a workaround, but I'm not sure how to pick a default when valid configurations are on a per-instance-basis. Please advise. > >> 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. http://incubator.apache.org/deltacloud/framework.html. Near the bottom where reboot_instance() and stop_instance() are documented. > >> 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 > >
