Thanks Padraig - made the changes, tested them and working fine, new
webrev, which applies cleanly against the gate.
http://cr.opensolaris.org/~jmr/pm_8324_local_search_using_api_29Jun_1210pm/
JR
Padraig O'Briain wrote:
John,
Just a couple of comments:
1) I think that __set_visible_status should not do anything if the
start page is showing. I suggest adding the following at line 1317
if self.w_main_view_notebook.get_current_page() != \
NOTEBOOK_PACKAGE_LIST_PAGE:
return
Otherwise descriptions are retrieved twice if category is clicked
immediately on startup.
2) Consider the following:
a) Start package manager
b) Switch to Search All Repositories
c) Switch back to Search Current Repository
d) Click on Accessories
Nothing happens!
I have no idea what is going wrong here. It looks like the filtering
is working as expected but the application list does not change.
I think the problem here is the following lines need to be added to
__save_setup_before_search:
self.saved_application_list_filter = \
self.application_list_filter
Padraig
Padraig
jmr wrote:
Thanks Brock, much appreciate the feedback. See comments below:
New webrev with your, Padraig's and Shawn's changes:
http://cr.opensolaris.org/~jmr/pm_8324_local_search_using_api_26Jun_5pm/
JR
Brock Pytlik wrote:
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.
Sure
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?
Well in Google the search time is reported in brackets at the top of
the page, but its just a nice to have. At present I wanted it there
to help with the debugging and optimization work for Search. Happy
to change it or take it out given feedback from users.
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?
I agree "current_not_show_repos" is misleading, renamed to
"current_repos_with_search_errors", which captures the intent here.
It was being unset in a number of places redundantly, it is now just
unset at the start of __parse_api_search_error() where it is
populated and then used in __handle_api_search_error.
When errors occur searching a repo the user is notified by a dialog
and a yellow warning triangle in the status bar:
__do_api_search
:
except api_errors.ProblematicSearchServers, ex:
self.__parse_api_search_error(ex)
gobject.idle_add(self.w_progress_dialog.hide)
gobject.idle_add(self.__handle_api_search_error)
The user can elect to not have these errors reported again in the
popup dialog. These repos to ignore on error are stored and are
persistent across invocations of PM. It does not mean that you won't
try and search these repos, only that the user won't be bugged with
errors telling them there are problems with it. What happens in this
case is that a yellow triangle is all they see in the statusbar if
there are errors on a given repo they have chosen to ignore.
Clicking on the triangle brings up the dialog listing all the repos
with errors and unsets the ignore state of these repos.
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 am leaving this until J's stuff lands and will rework, given his
and your recent proposal on:
http://defect.opensolaris.org/bz/show_bug.cgi?id=9670#c2
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.
I thought that's what we were doing:
[api.Query(" ".join(pargs), False, True, None,
None)], servers=servers
query_p.Query.__init__(self, text, case_sensitive, return_type,
num_to_return, start_point)
Searching for OFFice and office turn up 26 hits in opensolaris.org
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?
Good point - I like the idea of getting the results back
incrementally. I have submitted this as an RFE and will work on it
after this wad goes back.
9710 PM search should return results incrementally
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?
Done
Nits:
239: commented line
Done
266: extra whitespace
Done
1673: why not just pass self.is_search_all here instead of creating
search_all?
Done
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
_______________________________________________
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
