Darren,
LGTM.
- Dermot
On 05/30/12 09:34, Darren Kenny wrote:
Any takers on a second review?
The new webrev is at:
https://cr.opensolaris.org/action/browse/caiman/dkenny/7170813-1/webrev/
Thanks,
Darren.
On 29/05/2012 17:30, Darren Kenny wrote:
Thanks Jack...
Everyone else, I still need another review of this, if anyone is able to do
so...
Thanks,
Darren.
On 29/05/2012 17:24, Jack Schwartz wrote:
Hi Darren.
Thanks for adding the extra comment.
Thanks,
Jack
On 05/28/12 03:09 AM, Darren Kenny wrote:
Hi Jack,
On 25/05/2012 18:15, Jack Schwartz wrote:
Hi Darren.
Pretty tricky navigation in _validate_input(). :)
Looks like you are unshackling yourself from IPS constraints and doing
the checking yourself instead. Assuming this is OK; not an expert here.
Well, doing the checking ourselves allows us to present a more meaningful
error to the user rather than a Python stack trace.
Nit: I suggest adding a comment that set_image_args() allows for an
empty _publ.
I added some text to the doc string for set_image_args, so that it now
looks like:
def set_image_args(self):
'''Set the image args we need set because the information
was passed in via other attributes. These include progtrack,
prefix, repo_uri, origins, and mirrors. If we're creating a
zone image, we also need to set the use-system-repo property
in the props argument.
+
+ For the publisher used to create the image, it is possible to
+ omit the prefix (_publ) and allow IPS to do auto-discovery from
+ the repo itself.
'''
Thanks,
Darren.
Code LGTM.
Thanks,
Jack
On 05/25/12 09:02 AM, Darren Kenny wrote:
Hi,
Could I please get two reviewers for the fix for the following bug:
7170813 AI fails on client hits assert (prefix or refresh_allowed)
or not repo_uri
[ http://monaco.us.oracle.com/detail.jsf?cr=7170813 ]
The webrev is at:
https://cr.opensolaris.org/action/browse/caiman/dkenny/7170813/webrev/
The problem here is that the prefix for the IPS publisher was being omitted
and because I added the refresh_allowed=False in a previous fix, I ended up
causing an assertion to fire for this scenario - which is one that I hadn't
tested for.
It turns out that this omission of the prefix is only permitted for the
first publisher that is used at creation time of the install image - any
other publisher must specify the prefix - this is enforced by the IPS
client API.
So I added some code to also test this scenario, and generate a meaningful
error should the prefix be omitted from any additional publishers.
I verified that the scenario that caused the bug now works correctly, and
also that all the scenarios that cause the error to occur are correctly
caught.
I updated the test suite bug 7163961 to request for addition for tests for
these scenarios too.
Thanks,
Darren.
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
