Looks good to me. Minor nit on indent in beadm.py:
405 + gtk.MESSAGE_ERROR,
406 + True)
JR
Padraig O'Briain wrote:
I have spun another webrev,
http://cr.opensolaris.org/~padraig/ips-5197-v7/.
See below for further comments.
jmr wrote:
Ok - looks good, there are still some outstanding uses of private
api's which separate bugs should be logged for namely
misc.get_inventory() and img.get_root() call. I will be submitting a
webrev to sort out the fetching of descriptions on the fly. Few
comments:
I have logged bugs 9519 and 9520 and set bug dependencies correctly.
Great.
src/gui/modules/misc.py
def get_client_api_version():
41 47 """Return the current version of the Client API the
PM, UM and
42 48 WebInstall GUIs have been tested against and are
known to work
43 49 with.""" 44 - return 15 #
CLIENT_API_VERSION Used by PM, UM and WebInstall
50 + return CLIENT_API_VERSION
This func get_client_api_version() could be removed as its only used
in the get_api_object() call whichh now resides in
src/gui/modules/misc.py.
Yes. I have removed it in latest webrev.
Ok
See you have put in a generic handler for errors, can you check that
markup usage is being passed in correctly in all instances as this
stung us before, when some of the underlying Exceptions started
throwing exception strings containing '<' and '>'.
I have checked this. beadm.py is the only place where markup is
supported in the error dialog.
Great
I have changed each of the calls referred to below.
Ok
Padraig
In:
124 2122 def __get_application_categories_lists(self,
publishers=[]):
:
:
2145 2143 if not uptodate:
2146 2144 if first_loop == True:
2147 2145 first_loop = False
2148 2146
gobject.idle_add(self.setup_progressdialog_show)
2149 - self.__load_catalogs()
2147 +
self.api_o.refresh(pubs=[publisher], immediate=True)
Immediate=True is not required and should be removed. We only need
this is the user is doing an explicit refresh as in Reload.
In:
src/updatemanager.py
681 681 def get_updates_to_list(self):
:
697 + image_obj = self.api_obj.img
698 + self.api_obj.refresh(immediate=True)
Immediate=True is not required and should be removed.
In:
src/updatemanagernotifier.py
269 270 def check_for_updates(self):
:
278 + api_obj.refresh(immediate=True)
Immediate=True is not required and should be removed.
JR
Padraig O'Briain wrote:
I have respun the webrev,
http://cr.opensolaris.org/~padraig/ips-5197-v6/, to remove
immediate=True from call to refresh.
The webrev fixes
5197 gui should not manipulate image objects
The webrev contains the following changes:
1) Define error_occurred in misc.py and use in beadmin.py,
repository.py, webinstall.py, packagemanager.py, updatemanager.py
2) Define get_api_object in misc.py and use in webinstall.py,
packagemanager.py, updatemanager.py, updatemanagernotifier.py
3) Call refresh on api object instead of image.load_catalogs in
packagemanager.py
4) Use info call to get installed version in packagemanager.py
5) Use misc.get_inventory_list instead of Image.inventory in
updatemanager.py
Padraig
Shawn Walker wrote:
On Jun 16, 2009, at 3:02 AM, Padraig O'Briain wrote:
On 06/15/09 18:47, Shawn Walker wrote:
On Jun 15, 2009, at 8:50 AM, Padraig O'Briain wrote:
The webrev, http://cr.opensolaris.org/~padraig/ips-5197-v5/, fixes
5197 gui should not manipulate image objects
src/packagemanager.py:
line 3630: did you really want immediate=True here? When is
update_package_list called? Generally, immediate=True should be
reserved for cases where the user has explicitly requested a
refresh, otherwise the API should "do the right thing."
update_package_list is called from src/gui/modules/installupdate.c
after the user has installed or removed some packages. It is also
called when the user canceled the operation.
Is it safe to remove the immediate=True in this case?
It better be, or there is a bug. All immediate=True does is force
the system to actually contact the publisher's repository to see if
there is new metadata available instead of respecting the
refresh_seconds value that has been set for the publisher.
line 3652: while you should arguably be able to get this
information from a single info() call, for now, this would be
faster if you could call this for all of the packages you need to
get the installed version of instead of for each one.
We get the installed version for each package which was installed
or removed. I believe that the time taken to get the status for
each package individually is in the noise compared to the time to
install or remove the packages. For 9 packages it costs less that
.2 seconds.
Ah, ok. That's good to hear then. I was afraid it was taking much
longer than that.
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
