There is a new webrev for this available at:
http://cr.opensolaris.org/~richb/pkg-8048-v2/
which makes the changes suggested by Shawn[1] and Danek[2]
To be specific:
api_errors.py:
* BENameExistsException renamed to DuplicateBEName
* Doc string added to DuplicateBEName
* Adjusted the _() in DuplicateBEName to just be around the string part.
* Did the same for InvalidBENameException
bootenv.py:
* check_be_name no longer returns anything.
* Now raises InvalidBENameException if the be_name doesn't exist or
is invalid.
* Raises DuplicateBEName if the be_name already exists (and uses the
code suggested by Shawn (module the check for be_name of None which
is already done earlier).
api.py:
* Adjusted the check_be_name staticmethod here to return nothing.
* Fixed up the call to check_be_name in plan_install to not expect
anything to be returned.
Thanks.
[1] http://mail.opensolaris.org/pipermail/pkg-discuss/2009-April/012987.html
[2] http://mail.opensolaris.org/pipermail/pkg-discuss/2009-April/012990.html
-------- Original Message --------
Subject: [pkg-discuss] Code review: bug #8048 - image update doesn't
check to see if specified be-name already exists
Date: Wed, 22 Apr 2009 11:36:56 -0700
From: Rich Burridge <[email protected]>
To: pkg discuss <[email protected]>
Hi all,
Looking for a code review of the proposed changes for
the fix for bug #8048:
http://defect.opensolaris.org/bz/show_bug.cgi?id=8048
image update doesn't check to see if specified be-name already exists
Webrev is at:
http://cr.opensolaris.org/~richb/pkg-8048-v1/
I initially looked at overloading the InvalidBENameException
with an additional "message" parameter (default value of None),
which could be an alternate message.
The problem there was that check_be_name is a static method
(and I believe needs to be because of the way it's called in the
public API in api.py). In the end, I went for a new BE exception
BENameExistsException and added additional code into the
check_be_name method in bootenv.py.
I don't like that it replicates code that's in the class __init__
method, but I can't see another way of doing this because it's a
static method.
Brock (or others), maybe you can suggest a better approach...
Thanks.
_______________________________________________
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
