On 03/03/11 09:09, William Schumann wrote:
Ethan,
Please find responses inline. Took your suggestions unless otherwise
noted.
current webrev: http://cr.opensolaris.org/~wmsch/profile4/
differences with last webrev
http://cr.opensolaris.org/~wmsch/profile3-diff/
On 02/24/11 06:12 PM, Ethan Quach wrote:
William,
On 02/16/11 07:43, William Schumann wrote:
current webrev: http://cr.opensolaris.org/~wmsch/profile3/
differences with last webrev
http://cr.opensolaris.org/~wmsch/profile2-diff/
last webrev: http://cr.opensolaris.org/~wmsch/profile2/
TODO: I'm still hanging onto code that extracts hostname and
timezone from profiles until which time I'm sure that its removal
won't interfere with testing.
Curious. Does this code being left in include the current support
for the embedded SC profile? I'm noticing that's still in there and
it's something that should be removed by this project.
Indeed, the embedded SC profile code should be removed from the AI
client, since the AI client code is bound to the AI service image.
AI_database.py:
--------------------
260-264 - I don't get why choose to split up the functions like this:
numManifests() / numNames()
Why not just create:
numManifests() and numProfiles()
numManifests was used by existing code, so I kept it. There was no
need for a profile-specific version - the generalized (parameterized)
version was preferable.
That's fine then. I just thought it would have been better to hide the
tables names from anything outside of AI_database.py
so that the caller doesn't need to know about the defined table names?
to serve the code that is generalized with table name as a parameter.
Same comment goes for:
getManNames() / getNames()
getManNames - same as for numManifests
It seems you're doing that for:
getManifestCriteria() / getProfileCriteria()
so why not do it in other places?
because there was no need for a generalized entry point in this case.
275-276 - This comment needs updating.
655 - Should include the ":default" instance name here at least. but
preceding it with the scheme is also preferable,
"svc:/system/install/server:default"
publish_manifest.py:
--------------------------
59 - Why did you change this line. It looks correct the way it was.
missing open bracket for -C option.
The open bracket is not missing. Its meant to be [ -c | -C ]
-ethan
ai_get_manifest.py
------------------------
995 - is msg.get_filename() the name of the file as it exists on the
server? If so, I'm wondering if it'd be better to preserve that
naming on the installed system.
installadm.1m.txt:
-----------------------
439 - Shouldn't this line be {-m <manifest_name> | -p <profile_name>
...}
445 - "or or"
537-538 - With -P being provided to validate the given profile
against the service_bundle associated with some install service,
shouldn't the usage be:
installadm validate
-n <svcname> {-p <profile_name> ... | -P <profile_name> ...}
After you and discussed this, I implemented your suggestion. Modified
distro_const/checkpoints/pre_pkg_img_mod.py to copy
service_bundle.dtd.1 into the AI service image. Modified installadm
to require a service, then to look for the service_bundle in the AI
service image. Warns if service_bundle is not there.
Thank you,
William
584 - could you leave off the "assigned by DHCP" portion. Our
general netboot case uses DHCP, but we have a supported usage
scenario where DHCP isn't used, i.e. the wanboot without DHCP case.
In that case, the IP is manually set at the OBP, and is still what's
processed for the sake of Criteria here. So perhaps this can be
changed to:
ipv4 - IP version 4 network address
587 - Accordingly, can you change this to:
network - IP version 4 network number
597 - "manifest" -> "manifest or profile"
927-935 - Don't we need to specify the service name with -P?
Otherwise, what service_bundle are you validating against?
thanks,
-ethan
On 01/28/11 12:23 AM, Dave Miner wrote:
... set_criteria.py
87: Perhaps add a "not both" to the end here to clarify slightly
(though, I wonder, would it be possible and useful to be able to do
both at once? seems convenient.)
Added support for both at once.
Thank you,
William
_______________________________________________
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
