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

Revision history for this message
Raphaƫl Badin (rvb) 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.

Okay.

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

All right. It's probably worth a comment in the code then.

review: Approve

« Back to merge proposal