Jack,

I looked at webrev.srv.3.  Your doing a fantastic job.  Almost there.

Thanks,

John



================================================================================
usr/src/cmd/ai-webserver/AI_database.py
================================================================================
Looks good.
================================================================================
usr/src/cmd/ai-webserver/Makefile
================================================================================
Looks good.
================================================================================
usr/src/cmd/ai-webserver/cgi_get_manifest.py
================================================================================
Looks good.
================================================================================
usr/src/cmd/ai-webserver/delete_manifest.py
================================================================================
Looks good.
================================================================================
usr/src/cmd/ai-webserver/export.py
================================================================================
1. The comment for this code block does not match what is being done:

 109     # Find the file in the directory and cat it out.
 110     try:
 111         shutil.copyfile(options.mname, options.output_name)
 112     except IOError as err:
 113         print >> sys.stderr, "Error exporting manifest: %s: %s" % (
 114             err.strerror, err.filename)
 115         sys.exit(err.errno)
================================================================================
usr/src/cmd/ai-webserver/publish_manifest.py
================================================================================
1. The following could be an elif which would save a check in the case of
   options.criteria_c is being used:

 163         if options.criteria_file:

   Not a big deal if not changed.

2. Parentheses are not needed around the compound condition nor individual
   conditions below:

1479             if ((first_line.find(KSH93_SHEBANG) == -1) and
1480                 (first_line.find(PYTHON_SHEBANG) == -1)):

   could be:

        if first_line.find(KSH93_SHEBANG) == -1 and
           first_line.find(PYTHON_SHEBANG) == -1:

================================================================================
usr/src/cmd/ai-webserver/set_criteria.py
================================================================================
Looks good.
================================================================================
usr/src/cmd/ai-webserver/test/man_test_default.py
================================================================================
Looks good.
================================================================================
usr/src/cmd/ai-webserver/test/test_ai_database.py
================================================================================
Looks good.
================================================================================
usr/src/cmd/ai-webserver/test/test_export.py
================================================================================
Looks good.
================================================================================
usr/src/cmd/ai-webserver/test/test_publish_manifest.py
================================================================================
Looks good.
================================================================================
usr/src/cmd/ai-webserver/verifyXML.py
================================================================================
1. The parentheses are not needed below:

 132         if (ivalue < 0) or (ivalue > 255):

2. The parenthesis are not needed below:

 226             if (num_values != 1) and (crit_name == "cpu"):

================================================================================
usr/src/cmd/installadm/Makefile
================================================================================
Looks good.
================================================================================
usr/src/cmd/installadm/delete_service.py
================================================================================
1. The parenthesis around the compound conditional are not needed below:

624 elif (os.path.exists(os.path.join(image_path, "platform", "sun4u")) or 625 os.path.exists(os.path.join(image_path, "platform", "sun4v"))):
================================================================================
usr/src/cmd/installadm/installadm.py
================================================================================
Looks good.
================================================================================
usr/src/cmd/installadm/list.py
================================================================================
Looks good.
================================================================================
usr/src/cmd/installadm/properties.py
================================================================================
1. The parenthesis around the dictionary are not needed below:

 111         return (prop_dict[DEFAULT_MANIFEST_PROP])

================================================================================
usr/src/cmd/installadm/set_service.py
================================================================================
Looks good.
================================================================================
usr/src/cmd/installadm/setup-service.sh
================================================================================
1. The back slash is unnecessary and possibly in error:

 237                       "${SYS_AI_DEFAULT_MANIFEST}>"
 238                 default_manifest_src=${SYS_AI_DEFAULT_MANIFEST} \
 239         else

================================================================================
usr/src/cmd/installadm/test/test_set_service.py
================================================================================
Looks good.
================================================================================
usr/src/lib/install_common/__init__.py
================================================================================
Looks good.
================================================================================
usr/src/pkg/manifests/install-installadm.mf ================================================================================
Looks good.
================================================================================

On 04/ 5/11 05:19 PM, Jack Schwartz wrote:
Hi everyone.

Here is hopefully the last round of webrevs with significant changes for Derived Manifest. (Li is still testing but is almost done. He needs "final" bits to complete his testing.)

AI server changes are minimal, mostly fixes to bugs found by Li of QE.
vs previous webrev:
http://cr.opensolaris.org/~schwartz/110329.1/webrev.srv.3.4.diff/
vs slim_source:
http://cr.opensolaris.org/~schwartz/110329.1/webrev.srv.3/

AI client changes are a little more substantial, including bug fixes found by Li, re-whacking of aimanifest for better logging, and bugfixes to pathing.
vs previous webrev:
http://cr.opensolaris.org/~schwartz/110329.1/webrev.cli.2.3.diff/
vs AI-CUD gate*:
http://cr.opensolaris.org/~schwartz/110329.1/webrev.cli.3/

* Note: I pushed to AI-CUD gate at the time of first code review, so the AI-CUD team could use my bits for testing.

I would be grateful for the following people to help with the review as soon as possible. If you are named and can't review please let me know ASAP.

Client side:
Matt: changes to dmm.py and new dmm_log_test unit test.
Drew: changes to mim.py and mim_errors.py
Karen: changes to aimanifest.py and packaging
Ginnie: changes to logger.py
Note: Dave M. blessed changes to rbac files before going on vacation.

Server side:
John: all changes

    Thanks everyone for your time,
    Jack

P.S. Notes: AI client webrev shows changes for auto_install.py in the delta, but they are not my changes; ai_get_manifest.py is reverted to what was there before I pushed originally.





_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to