Brock, As requested, I've taken a look a look at your code review. Comments are below.
> http://cr.opensolaris.org/~bpytlik/ips-2670-v1/ client.py - line 1035: Is there a reason why we don't automatically rebuild the index if we know it has been corrupted? directory.py - line 51: use os.path.sep instead of "/" file.py - line 61: use os.path.sep instead of "/" link.py: - line 50: use os.path.sep instead of "/" actions/[directory | file | link]: - In each of these __init__() routines, you perform a re.search. What's the performance impact of performing a regular expression operation each time a file/directory/link action is created? Can you compile this re, instead of creating it anew each time? Is there another faster way of performing this check? api.py: - Is there any reason why all the calls to __reset_unlock() can't go in the finally clause where the activity lock is released? - Can you move remote_search() to client/retrieve.py? Is there any reason why it has to be in the api.py module? api_errors.py: - line 325: You're duplicating code that's elsewhere in the system. Check out TransportFailures in misc.py. If need be, we should promote some transport exceptions to ApiExceptions. IIRC, Shawn did some of this with his most recent putback. Keep in mind that I'm reserving the right to change any TransportExceptions in a future putback. However, it seems unwise to have multiple exceptions handling the same problem littered throughout the code. client/query_parser.py: - line 78: Is the granularlity of this lock correct? This is a class variable. All instances of TermQuery will use the same lock. Is it possible that different instances of TermQuery will be accessing different datasets? If so, they'll serialize unncessarily against a lock, even when they're referencing different datasets. indexer.py: - It looks like you need some I/O exception handling. The __close_sort_fh(), _add_terms(), _write_main_dict_line(), _generic_update_index(), _setup(), and _migrate() all look like they need some kind of EnvironmentError handling. I'm assuming we don't want to crash with a stack trace if files/directories get errors here. query_parser.py: - Would you please add more comments and docstrings to routines in this file? depot.py - lines 294-296: Remove XXX and code that has been commented-out -j _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
