[EMAIL PROTECTED] wrote:

> >   - line 159: merge this if with the actual lookup in a try/except block
> >     (and return if you catch a KeyError).
> 
> Testing this condition prevents us from raising an exception.  I thought
> code tended to perform faster if it didn't have to do exception
> handling.  (This is the case for C++ and Java).  I'm inclined to leave
> this as is.

Okay.  I think it's typically Pythonic to ask for forgivness rather than
permission, but I'm not sure what the actual performance implications are.
We haven't been terribly consistent here across the codebase; I'll leave it
up to you to make filelist more consistent or not.

> >   - line 494: I'm not sure that TransferInterruptedException is really the
> >     right name here.  TransferTimedOutException, maybe?  Or even just leave
> >     it as socket.timeout?  Unless you're using the same exception in other
> >     cases where a transfer was interrupted.
> 
> I want to catch the timeout and throw another exception so I can be sure
> that I've caught all of the cases where this exception might be
> generated by call sites in our code.  It'll be harder to debug a failure
> to handle this exception if I let a socket.timeout make it to the top.

Okay, fair enough.  Rethrowing with all our own exceptions might be a good
idea, in the end.

> If the name annoys you, I can change it to something else.

If you're going to use it for interruptions other than timeouts, keep it;
if it's just going to be timeouts (for the foreseeable future, at least)
then rename it.  IMHO.

> Thanks for reviewing this, sir.

Thanks for writing it!

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

Reply via email to