Code review comment for lp:~jtv/juju-core/session-certificate

Revision history for this message
Roger Peppe (rogpeppe) wrote :

On 20 June 2013 17:58, <email address hidden> wrote:
> Thanks Roger!
>
>
> On 2013/06/20 16:40:57, rog wrote:
>
>> i hope we manage to avoid the need for these temporary files sooner
>
> rather than
>>
>> later.
>
>
> Absolutely. There are several ways it might happen: by replacing
> libcurl with Go's built-in http library if it starts supporting this
> site, or by going through libcurl's in-memory mechanism for passing the
> certificates, or by storing the certificate somewhere slightly more
> durable perhaps. That's in descending order of desirability.
>
>
>
>> I'd probably just delete the defer and leave the
>> Delete in the NewManagementAPI error case.
>
>
> Done.
>
>
>
>> environs/azure/environ.go:178: // releaseManagementAPI frees up a
>
> context object
>>
>> for interfacing with Azure's
>> // releaseManagementAPI frees up an Azure management context.
>> ?
>
>
> I just made it say that it frees up the context object returned by
> getManagementAPI. Once. :)
>
>
>
>> i wouldn't mind seeing a test that checks that the temporary cert file
>
> is
>>
>> deleted when NewManagementAPI fails.
>
>
> There doesn't currently seem to be a failure path!

maybe just delete the error return from NewManagementAPI then
and code goes away :-)

« Back to merge proposal