Hi William,
I reviewed 'AI client' portion of changes. Please see
my comments/questions below.
Thank you,
Jan
general comments
- please update Copyright dates to <start_date>, 2011
ai_get_manifest.py
------------------
- it seems that file does not follow standard PEP8 formatting conventions -
please make sure it is addressed before push.
nit:
Copyright should be 2009, 2011
597 - comment should be corrected, 'cmd_options' should be perhaps
mentioned instead
of 'Args'
641 - should potential exceptions raised by os.unlink() be handled ?
643 - could you please enhance comment section with clarification of
purpose of input parameters ?
658 - could be str.endswith() used instead to make the code more readable ?
686-695 - could we take advantage of 'with open()' constructions here ?
691 - is Error log level appropriate here ?
795 - manifest_found variable seems redundant, could be removed
804 - AIGM_LOG.post() misses log level argument
814-822 - could we take advantage of 'with open()' constructions here ?
823 - in this comment section, could you please elaborate more on how
following algorithm works ?
auto_install.c
--------------
Since code for parsing SC manifest is no longer in use,
should we remove it - e.g. stuff in auto_parse.c file ?
I have one question, though. Due to the unsatisfied external dependences,
hostname and timezone SC parameters are still being configured by the
installer
itself (in ICT phase), not by new SC framework.
Would that still work even if the installer does not obtain that information
from SC manifest ?
auto-installer.c
----------------
OK
manifest-locator
----------------
OK
ict.py
------
- In original code,'svccfg apply -n' was called to validate generatedSC
profile.
That code was removed - is it intentional ?
- It seems like code allows several SC profiles land in /etc/svc/profile/
directory.
How is it assured they all get applied by smf(5) ?
I am asking, since I thought that smf(5) only applies well-defined set of
profiles
from /etc/svc/profile/ directory.
Right now, the installers utilize 'site.xml' one - it is created as a symbolic
link
to /etc/svc/profile/sc_profile.xml populated by ICT.
Or is it just an interim solution until CR6961214 is addressed ?
On 12/21/10 05:32 PM, William Schumann wrote:
Requesting code review for enhancements for service configuration
profiles in the Automated Installer.
http://cr.opensolaris.org/~wmsch/profile/
The code can be divided into sections:
UI - installadm invokes UI, modifications in database routines, since
profiles are now in a separate table.
CGI - runs under AI image-server under Apache, locate_profile,
ai-httpd.conf
AI client - modifications to ai_get_manifest, manifest-locator,
auto-install.c/h, ict.py
package modifications for new PY modules, CGI script:
install-installadm.mf
automated tests
Modified AI_database.py to allow many functions to specify database
table. Should default to 'manifest'. Changed AI_database.py message
output to stderr, which becomes significant for CGIs, where stderr
goes to the error log and stdout should be only for intended output so
that message header output is properly sequenced. Tried to fix a
problem that appears when there is no criteria in the manifest
database in build_query_str().
Design document:
http://hub.opensolaris.org/bin/download/Project+caiman/System+Configuration+Project/SCProfileProposal.odt
Authentication and user-defined scripting not supported in this release.
Decided to back out support for XInclude for the time being, problem
being that our best attempt at specifying when and if XInclusions are
to occur depends on modifications to service_bundle(4) DTD, and it
still needs approval by SMF. Some artifacts of this support remain in
the source, since approval is expected.
Have done unit testing on AI microroot, but have not built or tested
auto-install image. Will do so next, in concurrence with code review.
_______________________________________________
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
