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

Revision history for this message
Julian Edwards (julian-edwards) wrote :

On Monday 01 Jul 2013 06:58:25 you wrote:
> Review: Needs Information
>
> 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)

Yes, but I was going to leave it to another branch, it does not belong in this
one.

>
> [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.

Caller's responsibility. It's stored on the VM host *as supplied*.

>
> [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.

I found both to be more reliable.

« Back to merge proposal