Dan Price wrote:
> On Fri 11 Jul 2008 at 01:44PM, Brock Pytlik wrote:
>   
>> A pretty simple and straightforward change I think.
>> It splits the execute function in imageplan into two steps and moves the 
>> be/snapshot creation to be after the packages have been downloaded to 
>> disk, but before any actual actions have been performed.
>>
>> Here's the webrev:
>> http://cr.opensolaris.org/~bpytlik/ips-1249/
>>     
>
> Can you do me a favor and chmod src/client.py to be 644?  I don't know
> who is doing putbacks which are causing files to be 755, but it's
> annoying :)
>
> ---
> 483: shouldn't this say: "during install"?
> 599: shouldn't this say: "during uninstall"?
>
>   
doh, yes, fixed
> ---
> 600: please add one line of whitespace between 600 and 601
>   
done
> ---
> I assume you've tested this with e.g. ctrl-C during a download?
>   
Mhmm, I've ctrl-C'd on both the client and the server. I've also had the 
client act like it's getting timeout issues as well as just die with a 
run time error just after it entered the execute phase. I couldn't 
figure out how to integrate these things into the test suite, but I 
think I've covered the bases in testing on my machine.

> ---
> In your pair of exception handlers, you go to some effort to cope with
> "None", but it seems like the result will be:
>
>   An unexpected error happened during image-update:
>
> Perhaps at 383, 482 and 597, set tmp = "Unknown Error"
>
>   

So, the total error in the timeout cases for example looks like:

An unexpected error happened during image-update:

  Maximum number of timeouts exceeded during download.

As I reraise the exception, my inclination would be to just have these 
catches note where the error happened (once I've put in the fix above) 
then reraise and let the higher level exception handlers describe what 
happened. In short, I'm leaning more towards removing the %s in the 
error message completely now that I think more carefully think about it. 
How does that sound?

Brock
>         -dp
>
>   

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to