LGTM with a test added, also consider the logging severity.
https://codereview.appspot.com/13504044/diff/1/environs/manual/provisioner.go File environs/manual/provisioner.go (right):
https://codereview.appspot.com/13504044/diff/1/environs/manual/provisioner.go#newcode80 environs/manual/provisioner.go:80: logger.Warningf("failed to resolve %v: %v", ip, err) Is it really a warning if we can't do a reverse lookup? Is it something that we should be fixing later?
Perhaps just log with info or debug, certainly not really a big deal is it?
https://codereview.appspot.com/13504044/diff/1/instance/address.go File instance/address.go (right):
https://codereview.appspot.com/13504044/diff/1/instance/address.go#newcode64 instance/address.go:64: func HostAddresses(host string) (addrs []Address, err error) { How about a test for this?
https://codereview.appspot.com/13504044/
« Back to merge proposal
LGTM with a test added, also consider the logging severity.
https:/ /codereview. appspot. com/13504044/ diff/1/ environs/ manual/ provisioner. go manual/ provisioner. go (right):
File environs/
https:/ /codereview. appspot. com/13504044/ diff/1/ environs/ manual/ provisioner. go#newcode80 manual/ provisioner. go:80: logger. Warningf( "failed to resolve
environs/
%v: %v", ip, err)
Is it really a warning if we can't do a reverse lookup? Is it something
that we should be fixing later?
Perhaps just log with info or debug, certainly not really a big deal is
it?
https:/ /codereview. appspot. com/13504044/ diff/1/ instance/ address. go
File instance/address.go (right):
https:/ /codereview. appspot. com/13504044/ diff/1/ instance/ address. go#newcode64 address. go:64: func HostAddresses(host string) (addrs
instance/
[]Address, err error) {
How about a test for this?
https:/ /codereview. appspot. com/13504044/