Michal LGTM, I've reviewed the patch, built and ran it against the gate.
It resolves all the issues raised below, I see you added some support
for retrieve.DatastreamRetrievalError, didn't know we needed to catch
this one.
One thing to note though when I was testing this I compared it against
pkg search -r for timings and noticed that for the CLI you immediately
get back search results against the preferred repo, which I assume is
cached locally. Is the CLI doing a local search first and then a remote one?
Also the format of the error for servers not running search/1 is poor.
Can you log an API RFE against it thanks.
Some servers failed to respond appropriately:
http://pkg.opensolaris.org/contrib/:
http://pkg.opensolaris.org/contrib/ doesn't speak a known version of
search operation
http://osol-re.sfbay.sun.com/: http://osol-re.sfbay.sun.com/ doesn't
speak a known version of search operation
https://pkg.sun.com/opensolaris/extra/: Internal Server Error (500)
This looks fine above but it wraps badly in the Dialog. The Exception
should be changed to give us a list of tuples, <server>, <error_enum>,
<error string>
We could then process them and decide how best to present them to the
user, if at all (we could eat Internal Server Error for instance).
Perhaps along the lines of:
Packagemanager
----------------------------
The following repositories do not yet support Search All Repositories:
*contrib* (http://pkg.opensolaris.org/contrib/)
*opensolaris.org* (http://osol-re.sfbay.sun.com/)
JR
Michal Pryc wrote:
Hello,
I have prepared follow up which fixes those minor bugs:
http://cr.opensolaris.org/~migi/remote_followup_v1/
best
Michal
Michal Pryc wrote:
John,
That is fine. I am pushing to the gate. Because those are really
minor bugs I will do follow up asap.
best
Michal
jmr wrote:
Michal - built and tested. Looks really good. +1
There are a few minor nits that you should address in a follow up:
- Information for error while searching against repositories that
don't support remote search. This should be an Information dialog
not an Error one.
- Details still displayed after adding new repo and during
Refreshing Catalog, eventually cleared when list displayed. We
should clear at the same time as clearing the list and categories at
the start of Refreshing Catalogs
- We shouldn't select first row when coming back from a search, this
caused the Details info not to load for me, just switching to
another row sorted it though.
JR
Michal Pryc wrote:
Hey,
I have updated the webrev, so it should apply cleanly to the gate
and ran few smoke tests, I am planning to put this change Today to
the gate as the webrev was already posted more then 14 times?
http://cr.opensolaris.org/~migi/ips-6635-v15/
best
Michal
Padraig O'Briain wrote:
I have updated the webrev,,
http://cr.opensolaris.org/~padraig/ips-6635-v14/, to fix some
problems we found in testing this afternoon.
We believe that this webrev is good to commit after Brock's
changes have landed.
Padraig
On 03/10/09 13:36, Padraig O'Briain wrote:
This webrev, http://cr.opensolaris.org/~padraig/ips-6635-v12/,
applies to the changeset 929, i.e. the gate at the time of writing.
To test this, Brock's webrev
http://cr.opensolaris.org/~bpytlik/ips-2670-v2/, needs to be
applied to the gate in a separate workspace and SUNWipkg
installed from there. Note that there are two failures in
applying this patch.
src/packagemanager.py: set CLIENT_API_VERSION to 12
src/updatemanager.py: set CLIENT_API_VERSION to 12
The publisher which supports the search is http://ipkg.sfbay:40123.
These search operations are not cancelable.
It does not use api.info to get the status of the packages.
Padraig
_______________________________________________
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
