Hi Johansen,
Thanks alot for your inputs, please read my in-line comments.........
[email protected] wrote:
imageplan.py:
- lines 1026, 1028: Shouldn't be catching the exception here.
- lines 1029, 1030: This shoudl be handled in the CLI and GUI using a
top-level handler for the type of error that you're rasing.
I have made the changes for CLI (client.py) but I am too sure about the
way I need to handle this in GUI (packagemanager.py)
progress.py:
- lines 196-213: Instead of adding all of this as text to the
exception, I would create properties in the exception class for cur
and goal npkgs, nfiles, and nbytes.
- line 358: I updated the bug to reflect this, but
IncompleteDownloadException doesn't correctly capture what has gone
wrong here. The API told the progress tracker how much data it
expected to receive, but the numbers didn't match. Incomplete
downloads are okay, as long as they're not successful ones. I would
name this exception something like ProgressGoalMismatchException.
- lines 358-369: This doesn't belong in this file. It should be in
client/api_errors.py. The exception should be a subclass of
ApiError. I would change the exception to accept the cur and goal
counters from download done, and dynamically generate the error
message in __str__ based upon what counters are mismatched.
Done with these changes.
Question : Do we still need to log the errors here ? Or just raising the
exception would do ?
New Webrev : http://cr.opensolaris.org/~saurabhv/fix-10437-rev2/
pull.py:
- lines 721-726: You should be catching this at the top-level handler
and printing an error message there instead.
Done.
client.py and packagemanager.py: Need to update the GUI and CLI to
properly cope with this exception. You shouldn't be printing anything
from API code.
For CLI the exception would trickle down from
progress.py:download_done() --> modules/client/imageplan.py:preexecute()
--> modules/client/api.py:prepare() --> client.py:__api_prepare()
thus handling the exception at the topmost level (Is my understanding
correct? Or I should do something else here?)
Also as mentioned earlier, I am not too well versed with the GUI code,
thus please please advice on how this should be handled there
Thanks again,
~Saurabh
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
