Merge lp:~allenap/gwacl/remove-role-endpoints-2 into lp:gwacl

[Gavin Panella]

Niceties missed from the main remove-role-endpoints branch.

[Julian Edwards (julian-edwards)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

 review: approve

Approved but I'm interested to know why you changed a couple of things!

> defer func() { Info("Removing role input endpoint...") - //
> FIXME: Something above - AddRoleEndpoints seems a prime suspect - -
> // causes Azure to do something asynchronously, which we must wait
> for - // before attempting this call, or it will blow up
> with a 409.

Was this fixed previously then? Or was it not a problem?

> -func newX509Response() *x509Response { - return &x509Response{
> - Body: make([]byte, 0), - } -} -
[snip]
> - response := newX509Response() + response :=
> &x509Response{} response.StatusCode = httpResponse.StatusCode
> response.Body, err = ioutil.ReadAll(httpResponse.Body) if err !=
> nil {
>

This is removing a make() on the Body - is that right/ok? Go still
leaves me wondering whether it has changed the semantics when you
assign to Body in the second-last line here ... :/

Cheers.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlHx4mAACgkQWhGlTF8G/HdkIwCgpzEEtzP+ZKpCNC68JEo9+a9t
1csAn1YEXFNDv2Dx/Iz3qoUbMJP2Jnc2
=IDOH
-----END PGP SIGNATURE-----

review: Approve

[Julian Edwards (julian-edwards)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Sigh @ whatever screwed up formatting on the code quote parts.
/me stares at Thunderbird
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlHx42IACgkQWhGlTF8G/Hd6cQCgpixY0qeaTOaxUy1wiX/lcm79
vWEAoKWUMqX4a/BMOQu7BuisM8yykVXd
=xBSW
-----END PGP SIGNATURE-----

[Gavin Panella (allenap)]

>> defer func() { Info("Removing role input endpoint...") - //
>> FIXME: Something above - AddRoleEndpoints seems a prime suspect - -
>> // causes Azure to do something asynchronously, which we must wait
>> for - // before attempting this call, or it will blow up
>> with a 409.
>
> Was this fixed previously then? Or was it not a problem?

I added this in my previous branch, but also fixed it there and forgot
to remove this message.

>
>> -func newX509Response() *x509Response { - return &x509Response{
>> - Body: make([]byte, 0), - } -} -
> [snip]
>> - response := newX509Response() + response :=
>> &x509Response{} response.StatusCode = httpResponse.StatusCode
>> response.Body, err = ioutil.ReadAll(httpResponse.Body) if err !=
>> nil {
>>
>
> This is removing a make() on the Body - is that right/ok? Go still
> leaves me wondering whether it has changed the semantics when you
> assign to Body in the second-last line here ... :/

Yeah, it's not needed. The zero value for x509Response.Body is an
empty slice.

Thanks for the review!

=== modified file 'example/management/run.go'
--- example/management/run.go	2013-07-25 21:07:12 +0000
+++ example/management/run.go	2013-07-25 22:26:30 +0000
@@ -76,9 +76,6 @@
     api, err := gwacl.NewManagementAPI(subscriptionID, certFile)
     checkError(err)
 
-    // Exercise the API.
-
-    // Exercise hosted services API.
     ExerciseHostedServicesAPI(api)
 
     Info("All done.")
@@ -172,7 +169,7 @@
         deployments, err := api.ListAllDeployments(&gwacl.ListAllDeploymentsRequest{ServiceName: hostedService.ServiceName})
         checkError(err)
         if len(deployments) != len(detailedHostedService.Deployments) {
-            Infof(
+            Errorf(
                 "Mismatch in reported deployments: %d != %d",
                 len(deployments), len(detailedHostedService.Deployments))
         }
@@ -260,9 +257,6 @@
 
     defer func() {
         Info("Removing role input endpoint...")
-        // FIXME: Something above - AddRoleEndpoints seems a prime suspect -
-        // causes Azure to do something asynchronously, which we must wait for
-        // before attempting this call, or it will blow up with a 409.
         err := api.RemoveRoleEndpoints(&gwacl.RemoveRoleEndpointsRequest{
             ServiceName:    hostServiceName,
             DeploymentName: deployment.Name,
@@ -275,8 +269,8 @@
 
     if pause {
         var wait string
-        Info("Pausing so you can play with the newly-created VM")
-        Info("To clear up, type something and press enter to continue")
+        fmt.Println("Pausing so you can play with the newly-created VM")
+        fmt.Println("To clear up, type something and press enter to continue")
         fmt.Scan(&wait)
     }
 }

=== modified file 'x509dispatcher.go'
--- x509dispatcher.go	2013-07-25 17:31:17 +0000
+++ x509dispatcher.go	2013-07-25 22:26:30 +0000
@@ -65,12 +65,6 @@
     Header http.Header
 }
 
-func newX509Response() *x509Response {
-    return &x509Response{
-        Body: make([]byte, 0),
-    }
-}
-
 func performX509Request(session *x509Session, request *X509Request) (*x509Response, error) {
     Debugf("Request: %s %s", request.Method, request.URL)
     if len(request.Payload) > 0 {
@@ -89,7 +83,7 @@
         return nil, err
     }
 
-    response := newX509Response()
+    response := &x509Response{}
     response.StatusCode = httpResponse.StatusCode
     response.Body, err = ioutil.ReadAll(httpResponse.Body)
     if err != nil {