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
