environs/dummy: add Instances

environs/dummy: add Instances

* PROPOSAL *: Add the facility for dummy to return the actual 
environ.Instance objects that were returned. The goal is to be
able to identify if dummy started _your_ instance, not just _an_
instance. This could also be accomplished by returned the string of
instance.Id().

https://code.launchpad.net/~dave-cheney/juju-core/go-environs-dummy-add-instance-id/+merge/110730

(do not edit description out of merge proposal)

[dave_cheney.net, 2012-06-18 06:19:35]

Please take a look.

[niemeyer, 2012-06-19 01:32:32]

LGTM with the suggestion, but please wait for rog as he has been following this
closely.

https://codereview.appspot.com/6305105/diff/1/environs/dummy/environs.go
File environs/dummy/environs.go (right):

https://codereview.appspot.com/6305105/diff/1/environs/dummy/environs.go#newc...
environs/dummy/environs.go:60: Instances []environs.Instance
The idea of extending the operation with more details of what was actually done
sounds reasonable (except for the fact that mocking isn't great, but we're past
that point already).

That said, enriching the Operation type with every single field that may be used
by every single operation, and then having to know which operations set which
fields with which names, sounds a bit like we'll get in a messy situation very
soon.

We don't have to refactor it yet onto something more rich (like a type per op,
for example) but I suggest at least being more specific about what the field
means. I suggest something like StartedInstances and StoppedInstances for now,
so there's absolutely no doubts about what they mean and when they are set.
Then, let's watch to see how the pattern grows.

[rog, 2012-06-19 06:40:32]

LGTM as an interim measure but i think that gustavo's idea is good for moving on
from there.

https://codereview.appspot.com/6305105/diff/1/environs/dummy/environs.go
File environs/dummy/environs.go (right):

https://codereview.appspot.com/6305105/diff/1/environs/dummy/environs.go#newc...
environs/dummy/environs.go:60: Instances []environs.Instance
On 2012/06/19 01:32:32, niemeyer wrote:
> The idea of extending the operation with more details of what was actually
done
> sounds reasonable (except for the fact that mocking isn't great, but we're
past
> that point already).
> 
> That said, enriching the Operation type with every single field that may be
used
> by every single operation, and then having to know which operations set which
> fields with which names, sounds a bit like we'll get in a messy situation very
> soon.
> 
> We don't have to refactor it yet onto something more rich (like a type per op,
> for example) but I suggest at least being more specific about what the field
> means. I suggest something like StartedInstances and StoppedInstances for now,
> so there's absolutely no doubts about what they mean and when they are set.
> Then, let's watch to see how the pattern grows.

+1

[dave_cheney.net, 2012-06-19 06:47:59]

As discussed on IRC, i'll do a different proposal using interfaces.

On Tue, Jun 19, 2012 at 4:40 PM,  <rogpeppe@gmail.com> wrote:
> LGTM as an interim measure but i think that gustavo's idea is good for
> moving on from there.
>
>
>
> https://codereview.appspot.com/6305105/diff/1/environs/dummy/environs.go
> File environs/dummy/environs.go (right):
>
>
https://codereview.appspot.com/6305105/diff/1/environs/dummy/environs.go#newc...
> environs/dummy/environs.go:60: Instances []environs.Instance
> On 2012/06/19 01:32:32, niemeyer wrote:
>>
>> The idea of extending the operation with more details of what was
>
> actually done
>>
>> sounds reasonable (except for the fact that mocking isn't great, but
>
> we're past
>>
>> that point already).
>
>
>> That said, enriching the Operation type with every single field that
>
> may be used
>>
>> by every single operation, and then having to know which operations
>
> set which
>>
>> fields with which names, sounds a bit like we'll get in a messy
>
> situation very
>>
>> soon.
>
>
>> We don't have to refactor it yet onto something more rich (like a type
>
> per op,
>>
>> for example) but I suggest at least being more specific about what the
>
> field
>>
>> means. I suggest something like StartedInstances and StoppedInstances
>
> for now,
>>
>> so there's absolutely no doubts about what they mean and when they are
>
> set.
>>
>> Then, let's watch to see how the pattern grows.
>
>
> +1
>
> https://codereview.appspot.com/6305105/

[dave_cheney.net, 2012-06-19 21:38:00]

abandoned

https://codereview.appspot.com/6305105/diff/1/environs/dummy/environs.go
File environs/dummy/environs.go (right):

https://codereview.appspot.com/6305105/diff/1/environs/dummy/environs.go#newc...
environs/dummy/environs.go:60: Instances []environs.Instance
On 2012/06/19 01:32:32, niemeyer wrote:
> The idea of extending the operation with more details of what was actually
done
> sounds reasonable (except for the fact that mocking isn't great, but we're
past
> that point already).
> 
> That said, enriching the Operation type with every single field that may be
used
> by every single operation, and then having to know which operations set which
> fields with which names, sounds a bit like we'll get in a messy situation very
> soon.
> 
> We don't have to refactor it yet onto something more rich (like a type per op,
> for example) but I suggest at least being more specific about what the field
> means. I suggest something like StartedInstances and StoppedInstances for now,
> so there's absolutely no doubts about what they mean and when they are set.
> Then, let's watch to see how the pattern grows.

Yup, it could become a kitchen sink and goes against the idea of dummy being a,
so simple it couldn't possibly go wrong, provider. 

This branch was proposed because I misunderstood your comment about checking the
instance id that the provisioner creates, which has now been addressed, so the
pressure for this feature has been reduced.