jmr wrote:
I see Padraig's response on the icon issue which might be at the root
of your problems.
I have just applied the webrev against the latest gate, it had some
hunk offsets but was still ok, then did a full make install and make
packages. I have sorted Padraig's issue on the Mange Repositories
Reload as he had suggested and generated a new webrev against the
latest gate.
http://cr.opensolaris.org/~jmr/pm_8324_local_search_using_api_25Jun_4pm/
packagemanager.py:
271: since this is only used to track search's elapsed, time, maybe the
variable should include search in its name. As a bigger question, why
are we tracking and reporting the search time to the user? From a UI
perspective, I'm not sure why a user would care whether search took 7
seconds or 9 seconds. Is there a reason a user would want this info or
is it more a debugging/development tool for you?
I'm slightly confused about what the "current_not_show_repos" variable
means. On lines 807-810 it seems like those are exactly the ones you're
showing the user as having errors. Also, when does the variable get
cleared? If, and here I'm just guessing, it's a list of repos not to
search, does one failed search mean you won't retry that publisher again
until the next time packagemanger is restarted?
For lines 780-782, why are you getting the string of the error and
testing the string instead of looking at the class of the exception
using, for example, isinstance? That would give you far more control
over the information you display to the user as well as allowing you to
get the specific url, error code, message etc... Also, it would make
more explicit which errors you've accounted for and which are getting
ignored. For example, if I've followed the code correctly, if search
encountered a BadStatusLine error for a repo, the user would never be
notified of this problem, and would simply see that their search
returned no results.
I realize that a lot of this code will need to get rewhacked when J's
transport wad lands, but explicitly checking for specific errors and
handling them appropriately seems much closer to what the code will look
like in that world than the current approach.
1736: Why do case sensitive search by default? At least for command line
search, all the feedback we've gotten suggests that case insensitive
should be the default. Typically, if someone wants to search for
openoffice, they don't want to have to remember the exact capitalization
pattern used.
1740-1750 or so: I realize this hasn't changed in this fix,
I'm a bit confused about the approach taken here. Why create an
artificial limit on the number of results generated, why not treat this
as a generator and have it throw results to the user as soon as they're
ready? When we switched to that model on the cli, the user experience
was vastly improved because results started appearing much sooner. Also,
if you're going to limit the number of results shown to the user to be
100, why not tell the servers not to return more than 100 results?
1775, 1781: Why do insert 0, which is slow for long lists, and the
reverse the results? Can't this just be results.append(a_res), and then
you don't need to do the reverse either?
Nits:
239: commented line
266: extra whitespace
1673: why not just pass self.is_search_all here instead of creating
search_all?
Thanks,
Brock
JR
Shawn Walker wrote:
On Jun 25, 2009, at 6:04 AM, jmr wrote:
Hi , here is a webrev to move PM over to using the Search API for
all searches, both against single publishers (previously used
GtkTreeView filtering) and all publishers (added in 2009.06, already
using Search API).
webrev:
http://cr.opensolaris.org/~jmr/pm_8324_local_search_using_api_25Jun_1150am/
9442 Use Search API in PM for all searches
This will cause a slow down when searching against Single
Publishers. The timings for a search are listed in the Status Bar if
it takes longer than 1 sec. If a repo cannot be contacted there is a
time out of 30 secs, which is caught and reported as a failed search
against the repo. When we have better exceptions here from the
transport layer [9670] we can do a better job of handling the range
of errors.
Any idea why I see this when trying to start packagemanager from my
workspace?
Traceback (most recent call last):
File "./packagemanager.py", line 4032, in ?
packagemanager = PackageManager()
File "./packagemanager.py", line 265, in __init__
self.__register_iconsets(self.search_options)
File "./packagemanager.py", line 644, in __register_iconsets
iconset = gtk.IconSet(pixbuf) TypeError: pixbuf should be a GdkPixbuf
I've blown away my proto area and done a make install, and I've
checked that my PACKAGE_MANAGER_ROOT is set right.
I'm on build 111b here.
Cheers,
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
