Nirmal: Might be good to check with pylint as well to make sure things are OK.
----- Original Message ----- From: [email protected] To: [email protected] Cc: [email protected] Sent: Wednesday, April 11, 2012 3:57:28 PM GMT -08:00 Tijuana / Baja California Subject: Re: [caiman-discuss] code review request for CR 7108281 On 04/11/12 03:41 AM, Nirmal Agarwal wrote: > Hi all > > Can I please get the code review for CR 7108281 > > 7108281 create RBAC execution profiles for installadm > > Webrev : > > https://cr.opensolaris.org/action/browse/caiman/nirmal27/7108281-rev0/webrev/ > <https://cr.opensolaris.org/action/browse/caiman/nirmal27/7108281-rev0/webrev/;jsessionid=9662DF16CB545E8304942C4DC3F2DA3A> Hi Nirmal, In your manual testing, did you test installadm-convert?? I don't see it listed. Same question for validate_profile and export. Also, did you test the commands with both with a profile with proper privileges and one that didn't have them? installadm_common.py 205 I think it would be better to raise some other exception from check_auth and then have the various subcommands do the try/except and raise the SystemExit if desired. 208 I'm not seeing where this is done? 205 and/or 226: Is it possible to add a unit test for this in test_installadm_common.py? test_create_profile.py 47 alphabetize import 221 typo: "do noting" 503 should this be reverted in the tearDown method? create_client.py 37 this line should be moved back to where it was as it is an internal module delete_client.py 28 Don't think you need to import os anymore delete_service.py 28 same here Thanks, Sue > As a part of this CR, following authorizations for installadm will be > introduced. > > 1) solaris.installadm.client:RO::Administer Automated Install Clients:: > 2)solaris.installadm.manifest:RO::Administer Automated Install Manifests:: > 3)solaris.installadm.profile:RO::Administer System Configuration Profiles:: > 4)solaris.installadm.service:RO::Administer Automated Install Services:: > > Testing : > Pep8 clean > Slim test results : > /net/indiana-build.us.oracle.com/export/home/na210770/ai/rbac/slim-test > Manual Testing : > Below commands were tested : > --> create-service > --> create-service -n /export/home/test2 -i 192.168.56.4 -c 2 -n test2 > --> create-service -d /export/home/test3 -i 192.168.56.7 -c 2 -n test3 -s > /var/tmp/sol-11_1-12-ai-sparc.iso > --> create-service -t test3 -n test4 > --> create-manifest -m test5 -n test4 -f > /usr/share/auto_install/manifest/default.xml > --> set-criteria -m test5 -n test4 -c hostname="test.com" > --> update-manifest > --> delete-manifest > --> set-criteria -p > --> create-profile > --> delete-profile > --> create-client > --> delete-client > --> disable (service) > --> enable (service) > > PSARC detail : PSARC/2012/139 FastTrack ( under review) > > > Ethan : Thanks for all the guidance > > Thanks > Nirmal > > > > _______________________________________________ > 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
