On Thu, Jul 24, 2008 at 10:02:32AM -0700, Brock Pytlik wrote:
> I'd especially appreciate comments on the changes in
> server/query_engine.py and the changes in manifest.py which appeared in
> v1. (Sorry, I don't have a diff for v1 -> v2, but I'll make sure I can
> generate that from now on.)
>
> http://cr.opensolaris.org/~bpytlik/ips-search-v2/
So I just have a style suggestion at this point. I think it may be clearer to
abstract some of the functionality in server/query_engine.py. Makes it easier
to see what's going on (to me at least).
For example, if you abstract the opening, reading, and closing dicts pieces
into functions like:
def open_dicts(raise_on_none=False):
if ss.consistent_open(self._data_dict.values(),
self._dir_path, FILE_OPEN_TIMEOUT_SECS) == None:
if raise_on_none:
raise
search_errors.NoIndexException(self._dir_path)
return False
return True
def read_dicts():
for d in self._data_dict.values():
if d == self._data_main_dict:
continue
d.read_dict_file()
def close_dicts():
for d in self._data_dict.values():
d.close_file_handle()
Then the search function (currently this):
def search(self, query):
""" Searches the indexes in dir_path for any matches of query
and returns the results.
"""
self.dict_lock.acquire()
try:
if ss.consistent_open(self._data_dict.values(),
self._dir_path, FILE_OPEN_TIMEOUT_SECS) == None:
raise
search_errors.NoIndexException(self._dir_path)
try:
for d in self._data_dict.values():
if d == self._data_main_dict:
continue
d.read_dict_file()
matched_ids, res_ids =
self.search_internal(query)
finally:
for d in self._data_dict.values():
d.close_file_handle()
return self.get_results(res_ids)
finally:
self.dict_lock.release()
Becomes:
def search(self, query):
""" Searches the indexes in dir_path for any matches of query
and returns the results.
"""
self.dict_lock.acquire()
try:
self.open_dicts(True)
try:
self.read_dicts()
(_, res_ids) = self.search_internal(query)
finally:
self.close_dicts()
return self.get_results(res_ids)
finally:
self.dict_lock.release()
Similarly, the piece in __init__ becomes:
try:
if self.open_dicts():
try:
self.read_dicts()
finally:
self.close_dicts()
finally:
self.dict_lock.release()
Anyhow, I'll post more comments as I make more progress into the review (sorry
for taking so long).
Thanks,
Brad
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
