On Tue, Oct 20, 2009 at 11:00:02AM -0700, Danek Duvall wrote: > [email protected] wrote: > > > On Tue, Oct 20, 2009 at 10:25:58AM -0700, Danek Duvall wrote: > > > > http://cr.opensolaris.org/~swalker/pkg-cat-p3/ > > > > > transport.py: > > > > > > - line 376: Why this test? > > > > The client has to compute the succesful requests if failures occurred. > > Otherwise, all requests are assumed to have succeeded. > > I guess what I was seeing was that success would be computed correctly by > the filter regardless of whether failedreqs was empty or not, and having > flist be the empty list rather than None wouldn't change the way the rest > of the code worked (and if you ever chose to iterate over it, you wouldn't > have to test to see if it were None).
My concern here was that flist may be arbitrarily long. Iterating over the list when there are no failures seemed wasteful. I don't have a problem with changing the way the test is performed if you're opposed to the len() call. > > > - line 377: success = [ x for x in flist if x not in failedreqs ] ? > > > > What's wrong with filter? > > It changes in py3k, returning an iterator instead of a list. Between > compatibility with the future and consistency in our code, I'd rather see > the comprehension, particularly since it's really not a hard one to read. Ok. Will this be true for map() and reduce() as well? > > > - line 414: > > > > > > if failedreqs and failures: > > > failures = [ x for for x in failures if x.request in > > > failedreqs ] > > > tfailurex = tx.TransportFailures() > > > tfailurex += failures > > > raise tfailurex > > > > The TransportFailures object coalesces re-occuring errors. I don't > > believe that the += is going to work. > > Ah. TransportFailures objects have a special append() method, rather than > inheriting from list objects? Fair enough. The two tests could still be > improved. No objection to changing the conditional or the list comprehension. -j _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
