Code review comment for lp:~thumper/juju-core/verify-storage-move
Revision history for this message
| Roger Peppe (rogpeppe) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
bbfa235
(Get the code!)
I have reservations about this.
I don't think we should be further entrenching the arbitrary way in
which the ec2 provider happened to mark its server instances.
Some thoughts below.
https:/
File environs/
https:/
environs/
layering violation - providers
On 2013/10/03 03:38:01, thumper wrote:
> On 2013/10/03 03:21:00, axw1 wrote:
> > Is this TODO no longer applicable? Seems to be...
> I took it out because everything expects it to be there.
> It is how bootstrap checks to see if the environment
> is already bootstrapped. I think for consistency across
> providers this comment no longer makes sense.
The comment *is* still relevant IMHO. It has long been
the plan to move away from using file storage on ec2 to
indicate state servers (it could be done nicely with
instance tags) and therefore allow the possibility of
having an environment with no local storage at all.
It is *currently* how bootstrap checks to see if the
environment is already bootstrapped, but there's no
particular necessity for it to be this way.
It would be better to have an Environ.
call instead.
https:/
File environs/
https:/
environs/
environs.Environ) error {
We usually use the word "ensure" to mean "ensure that something is so".
That's not the case here.
I'd prefer:
// IsBootstrapped returns whether the environment is
// bootstrapped.
func IsBootstrapped(env environs.Environ) (bool, error)
https:/
File environs/open.go (right):
https:/
environs/
s/ error//
https:/