Thanks for the feedback Brock, much appreciated, comments below:
JR
Brock Pytlik wrote:
> I've only looked at updatemanager.py, and mostly focused on how it
> interacts with the API.
>
> line 329-300:
> You might want to pass a function as the fourth argument to the
> constructor here. The function you provide there will be called
> whenever the cancelable status of the api changes. That would let you,
> for example, flip a cancel button as active or non-active as the back
> end moves through different stages. It's also possible that such
> functionality isn't needed, in which case we'll remove that call back
> function in the future. This might remove the need for you to
> introduce lines like 886 throughout the code and instead rely on the
> callback mechanism.
Great will make the change, I was checking when the user clicked on
cancel, this is much better.
>
>
> 820: I'm curious why you're installing without refreshing the catalogs
> in the default case.
The pkg refresh will have been called from the installed cron job (runs
update-refresh.sh periodically, calls pkg refresh), so I did not want to
call it again. If you think I should I certainly can. Would the best
place to do the refresh be in __get_image_obj_from_directory( ) where I
am fetching the catalogs initially?
>
> 821-837, 864-880, 888-908:
> Unless you're calling out those API exceptions specifically, it might
> be easier to maintain and read if the code looked something like:
Ah ok will make the change - I was just trying to get a handle on what
could be emitted and then take perhaps different actions if need be. For
now I just log the error , so the general ApiException is certainly cleaner.
>
> except (api_errors.CanceledException):
> gobject.idle_add(self.w_progress_dialog.hide)
> self.__cleanup()
> return
> except (api_errors.ApiException), ex:
> self.install_error = True
> gobject.idle_add(self.__progress_error_eval)
> gobject.idle_add(self.__update_progress_info,
> self._("\nERROR"), True)
> gobject.idle_add(self.__update_progress_info, \
> self._("Update Evaluate failed:\n%s" %
> ex))
> self.__cleanup()
> return
>
> 849: Probably should either use _extract_names here or get rid of the
> method.
>
> I was surprised that updatemanager is only installing packages, and
> never doing an image-update, but I probably don't have a great
> understanding of the use case for it.
This is a first step. I would like to have an Update All button as well.
The problem is that I'd really like to be able to tell if a package is
incorporated when we get the initial list, but I cannot do that at
present (the info object has a state field with incorporated in it but
its not being filled in). If I could then I could indicate this in a
status column and only enable Update All when an incorporated package(s)
was selected. A much better user experience I think. So work in progress :)
JR
>
> Take care,
> Brock
>
>
> jmr wrote:
>> Hi,
>>
>> Here is a webrev of the first cut of the UpdateManager for
>> OpenSolaris. Its using Brock's new API, including Michal's
>> ProgressTracker changes. It will need refreshed when these other
>> patches land in the gate.
>>
>> http://cr.opensolaris.org/~jmr/updatemanager_v2_Oct03/
>>
>> It consists of:
>> - service svc:/application/pkg/update, responsible for adding a cron
>> job entry to launch update-refresh.sh periodically (as already
>> discussed on the list, workaround as there is no cron job action in IPS)
>> - update-refresh.sh: runs pkg refresh as determined by the cron job
>> entry
>> - updatemanagernotifier: a desktop startup application that checks
>> for updates periodically (Daily, Weekly, Monthly) and notifies user
>> in the notification panel by displaying an update available icon.
>> Clicking on the notification icon launches updatemanager
>> - updatemanager: updatemanager GUI
>>
>> There is some pylint work to do, a few changes in
>> updatemanagernotifier and desktop icon to be added for updatemanager,
>> but this will let those interested have a look.
>>
>> When built SUNWipkg-um is the generated package. To get
>> updatemanagernotifier running requires a reboot. You also need to
>> have SUNWpython-notify installed (coming into b99).
>>
>> We are targeting this for the 2008.11 release.
>>
>> JR
>> _______________________________________________
>> 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