>
> > +
> > +def file_output(cap, path_list, newest_first = False):
> > + """
> > + If newest_first is True, the list of files in path_list is sorted
> > + by file modification time in descending order, else its sorted
> > + in ascending order.
> > + """
> > if cap in entries:
> Cap seems a bit overloaded, wasn't sure whether it meant max size allowed
> or capture. Context undoubtably makes this obvious
>
>
its needed because file_output is called from places other than tree_output.
> > - for p in path_list:
> > - if os.path.exists(p):
> > - if unlimited_data or caps[cap][MAX_SIZE] == -1 or \
> > - cap_sizes[cap] < caps[cap][MAX_SIZE]:
> > - data[p] = {'cap': cap, 'filename': p}
> > - try:
> > - s = os.stat(p)
> > - cap_sizes[cap] += s.st_size
> > - except:
> > - pass
> > - else:
> > - output("Omitting %s, size constraint of %s
> exceeded" % (p, cap))
> > + path_entries = [(path, os.stat(path)) for path in path_list if
> os.path.exists(path)]
> Does this exceed 80?
>
> Yes. I didn't see this style applied in this file, so I was staying
consistent with the overall style.
Anyway this has changed that it shouldn't be an issue.
> > + path_entries.sort(cmp=fcmp, reverse=newest_first)
> To follow on Ben's note, a preferred version would probably be
>
> mtime = lambda (path, stat): stat.st_mtime
> for p in sorted(path_entries, key=mtime):
>
Changed. But I prefer to apply the sort using list.sort() since this sorts
the elements in place
sorted() copies the original list.
> > + for p in path_entries:
>
> > + if unlimited_data or caps[cap][MAX_SIZE] == -1 or \
> > + cap_sizes[cap] < caps[cap][MAX_SIZE]:
> > + data[p] = {'cap': cap, 'filename': p[0]}
> > + try:
> > + cap_sizes[cap] += p[1].st_size
> > + except:
> Why the unguarded except? This is asking for all kinds of trouble
>
Removed. It exisited pre my change. I should've removed it earlier.
>
> > + pass
> > + else:
> > + output("Omitting %s, size constraint of %s exceeded" %
> (p[0], cap))
> >
> > -def tree_output(cap, path, pattern = None, negate = False):
> > +def tree_output(cap, path, pattern = None, negate = False, newest_first
> = False):
> Not your trend, but style disapproves of spaces around default arguments
> here.
>
>
Fixed.
> > + """
> > + Walks the directory tree rooted at path. Files in current dir are
> processed
> > + before files in sub-dirs.
> > + """
> > if cap in entries:
> > if os.path.exists(path):
> > - for f in os.listdir(path):
> > - fn = os.path.join(path, f)
> > - if os.path.isfile(fn) and matches(fn, pattern, negate):
> > - file_output(cap, [fn])
> > - elif os.path.isdir(fn):
> > - tree_output(cap, fn, pattern, negate)
> > + for root, dirs, files in os.walk(path):
> > + fns = [f for f in [os.path.join(root, f) for f in
> files] \
> > + if os.path.isfile(f) and matches(f, pattern,
> negate)]
> Duplicate use of 'f' makes me nervous here. Seems like precipitous code,
> to make up a phrase.
>
>
Fixed.
> > + file_output(cap, fns, newest_first)
> Style nit, use newest_first=newest_first (as with any kwarg, to guard
> future position changes)
> >
> > def func_output(cap, label, func):
> > if cap in entries:
>
> I've forgotten bugtool's structure/config. I assume this has logic
> elsewhere to split up the size quota into smaller chunks, so the global max
> couldn't be taken up by the first giant file it comes to.
>
>
I haven't digged too deep. From what I have seen, quotas are per CAP_ or
Plugin. For a given plugin a very large file will eat up all the quota
preventing anything else from being added.
> Again, sorry for any typos or lack of context, composed via phone.
>
> -Reid
>
> > --
> > 1.7.2.5
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
