On 01/25/11 04:14 PM, John Fischer wrote:
Dave,
Here is the webrev that includes only your version and ai_get_manifest.py
changes:
http://cr.opensolaris.org/~johnfisc/webserver/webrev-webserver-design-dave-diff/
I'm skeptical that the comment on the version file correctly captures
its anticipated scope. A protocol version is perhaps inferred from the
image version at present, but proper protocol versioning likely involves
encoding versions in the protocol (as an argument, part of a URL, etc.).
I'd strip the comment down to just something like:
This file provides version information related to Automated Installation.
[A nit is that saying it's the ISO image version is slightly incorrect;
"Boot image version" would be more correct as it's not the ISO that's
being run]
Dave
Thanks,
John
On 01/25/11 12:23 PM, Dave Miner wrote:
On 01/24/11 06:23 PM, John Fischer wrote:
Dave,
Thanks for the review. In short, I'll add verbage to the version file
and print an error and
exit for the second issue. Do you want to see the code in another
webrev?
I'd like to see the comment that you're planning to put in the version
file.
Dave
Thanks,
John
On 01/24/11 11:56 AM, Dave Miner wrote:
My request for explanatory comments in
usr/src/cmd/auto-install/version has not been addressed.
I totally missed that comment in round 4 of the review. Sorry.
I'll add some text within the file and for the variable specifically.
ai_get_manifest.py, 612: If no version is found in the file, is 1.0
really the right thing to return? Why wouldn't we error this case?
My thinking was this is the first version of the protocol change. If the
file exists
then we are at least at version 1.0. We could:
1. return 1.0 - based upon logic above, perhaps print a warning
2. return None - passing None within the http request will cause the
server to return an
error.
3. return an error and exit
4. print an error and exit
I think that I'll print and error and exit. This should really never
happen.
Dave
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
