Hi,
jmr wrote:
> Michal - few comments,
>
> What's the purpose of:
>
> 1444 + self.__catalog_refresh_done()
To shut down the bouncing progress and reload list, without doing that
the bouncing progress will be bouncing and the model will be detached
from the treeview.
>
> If you have a permissions error should you not just be returning -1 here
> or is this just a status flag indicating the end of the operation
> regardless of failure or success?
This is flag used by other part of application:
1324 1324 ret = self.__catalog_refresh(False)
1325 1325 if ret != -1:
1326 1326 self.ips_uptodate =
self.__ipkg_ipkgui_uptodate()
1327 1327 else:
1328 1328 self.progress_canceled = True
1329 1329 self.progress_stop_timer_thread = True
and I didn't wanted to change this, since this bug was not about that
return value (I want to keep the webrevs connected with the bugs at this
stage and focus only on fixing really important bugs) I will put this on
TODO list after 2008.11.
> I would prefer to see this using an explicit exception as all of the
> rest of the PM and UM code does.
>
> 379 + except RuntimeError, ex:
> 380 + if "no defined authorities" in
> str(ex):
I really don't want to change that, since this was tested and works
well. Once we will have more precise exception
api_errors.AuthorityException ? then we can remove that.
> If we are never going to hit this error then it should be removed and a
> top level Exception handler added to catch any unexpected errors.
>
> 399 + except RuntimeError, ex:
> 400 + #The "no defined authorities" should
> never happen
> 401 + #Because we skipped removal of repository
> during name
> 402 + #change as we disabled name changing.
> 403 + if "no defined authorities" in str(ex):
> 393 404 pass
> 394 405 else:
> 395 406 err = str(ex)
> 396 407
> self.__error_with_reset_repo_selection(err)
> 397 408 return
See the comment above.
>
> You will need to check with Kelly to see if there is a registration URL
> that should be added here:
>
> 335 + msg = self.parent._("Accessing
> this restricted repository failed." \
> 336 + "\nYou either need to
> register to access this repository," \
> 337 + "\nthe certificate expired,
> or you need to accept the repository" \
> 338 + "\ncertificate.")
This is subject to change. If I will not get new string by Tomorrow I
will push with this string.
best
Michal
>
>
> JR
>
> Michal Pryc wrote:
>> Hi,
>>
>> The webrev:
>> http://cr.opensolaris.org/~migi/13_10_2008_bugs_4987_4990_v1
>>
>> After adding each change I was testing if it works by raising this
>> error or unplugging the network cable if I was testing network
>> problems, so this should work fine.
>>
>> The bugs:
>>
>> 4990 Disable Repository rename
>> URL: http://defect.opensolaris.org/bz/show_bug.cgi?id=4990
>>
>> Lines in the code for this change (line numbers taken from the changed
>> file):
>> (93 - 95), (357-359)
>> http://cr.opensolaris.org/~migi/13_10_2008_bugs_4987_4990_v1/src/gui/modules/repository.py.wdiff.html
>>
>>
>> On the line 380 there is still "no defined..." string comparison, but
>> we will never hit this as we disabled renaming of the repository and
>> we will never go through the line (360)
>> Similar situation is for line 400-403, we might hit those lines, but I
>> didn't remove this check to not introduce too much risk.
>>
>>
>>
>> 4987 Improve error messages for PM
>> URL: http://defect.opensolaris.org/bz/show_bug.cgi?id=4987
>>
>> The resto of the changes:
>>
>> http://cr.opensolaris.org/~migi/13_10_2008_bugs_4987_4990_v1/src/packagemanager.py.wdiff.html
>>
>>
>> line 1441-1445, because we are running with gksu this can only happen
>> while user will start from the terminal without gksu and will try to
>> edit repositories, then we are refreshing catalogs of course this will
>> fail as the user doesn't have sufficient privilages but we already
>> reported this while editing repositories. This is the only way of
>> hitting those lines at the moment. Properly we shouldn't allow user to
>> edit repositories in the first place, but such change will introduce
>> too much risk in this stage. This is something to work on for the next
>> release.
>>
>> line 1472 sometimes errors doesn't have data then this will fail so we
>> need to check if data is in the error variables before checking if
>> data contains any information.
>>
>> 1478-1479 we agreed to have nice info icon instead of error one for
>> network problems.
>>
>>
>> http://cr.opensolaris.org/~migi/13_10_2008_bugs_4987_4990_v1/src/gui/modules/installupdate.py.wdiff.html
>>
>>
>> 42-44 and 515-516 I have checked to unplug the cable in each stage of
>> the installation process and this needs to be added to show proper
>> network problems dialog while we are downloading files.
>>
>> 368-369 If there is any question at the end of the error dialog we are
>> showing nice blue "?" icon instead of red error sign.
>>
>> 339-340, 425-426, 457-458, If there is no question and the error is
>> information to the user rather then error I've changed to show nice
>> "i" icon rather then red error sign.
>>
>> 473-474 we need to check weather the args exists before we will take
>> first argument - this failed when I was testing errors, that is why it
>> is in that place.
>>
>> 495-507 I've splitted those two errors. The second one
>> PlanMissingException will have detailed information in the text details.
>>
>> I was not changing the PlanCreationException for all actions, because
>> they are fine, basically we wanted to have (example for Install/Update
>> action) "Install/Update failure"with additional details view it is
>> like this in the code. Somehow I've send wrong error message in
>> yesterdays e-mail telling about network problem in the
>> PlanCreationException:
>> except api_errors.PlanCreationException, e:
>> err_msg = self.parent._("Install/Update failure" \
>> " in plan creation.")
>> err_text = str(e)
>> gobject.idle_add(self.__error_with_details, \
>> err_msg, err_text)
>>
>>
>> http://cr.opensolaris.org/~migi/13_10_2008_bugs_4987_4990_v1/src/gui/modules/repository.py.wdiff.html
>>
>>
>> Most of the errors were changed to question icons or info icons the
>> error ones are the real errors.
>>
>
> _______________________________________________
> 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
