Code review comment for lp:~julian-edwards/gwacl/user-data-config

Revision history for this message
Raphaƫl Badin (rvb) wrote :

Looks good, but I've got a question (see [1]), hence "needs info".

[0]

46 + var wait string
47 + fmt.Println("Pausing so you can play with the newly-created VM")
48 + fmt.Println("To clear up, type something and press enter to continue")
49 + fmt.Scan(&wait)

That's great (I was planning to do something like that myself today :)). How about printing more details about the host to help the testing:

    request := &gwacl.GetDeploymentRequest{ServiceName: hostServiceName, DeploymentName: machineName}
    deploy, err := api.GetDeployment(request)
    fqdn, err := deploy.GetFQDN()
    fmt.Println("host:", fqdn)
    // The user name should be a variable!!!!
    fmt.Println("user:test")
    fmt.Println("password:", password)

[1]

138 + <UserData>%s</UserData>

What about escaping? Shouldn't this be base64 escaped or do you think it should be the caller's responsibility to do something like that?
My take on this is that gwacl should probably base64-encode this to avoid messing with the XML structure.

[2]

24 + location := "West US"
25 + release := "13.04"

Why this change? I don't mind the location change but precise is our target platform at the moment so I'd rather keep that as our default for experimenting.

review: Needs Information

« Back to merge proposal