Looks ok to me now.
Sue
On 04/19/12 08:10 AM, Nirmal Agarwal wrote:
Hi Sue, Takeshi-san
Thanks for the review.
Please find the updated webrev which fixes the localization.
webrev rev 3:
https://cr.opensolaris.org/action/browse/caiman/nirmal27/7108281-rev3/webrev.rev3/
webrev diff with rev 2 :
https://cr.opensolaris.org/action/browse/caiman/nirmal27/7108281-rev3-diff/webrev.rev3-diff/
Source is Pep8 clean and Pylint output is unchanged.
I verified the creation of solaris_install_common.po in the proto area.
On 04/18/12 19:58, Sue Sohn wrote:
Hi Nirmal,
General for all the subcommands calling check_auth():
I don't think that:
except UnauthorizedUserError as err:
raise SystemExit(_(err))
will work for translation. How would gettext know to extract the string
in install_common/__init__.py.src for it to be translated?
export.py(88) and set_criteria.py(101)
If the user requests export of both manifest and profile, but only has
auth for one, the command will exit. How will the user know which
authorization is missing, profile or manifest?
fixed. Now the message displays which auth is missing.
Thanks
Nirmal
Sue
On 04/17/12 10:33 AM, Nirmal Agarwal wrote:
Hi Sue, Karen
Thanks for review,
I have fixed comments provided.
Please find the updated webrev :
diff webrev :
https://cr.opensolaris.org/action/browse/caiman/nirmal27/7108281-rev1-diff/webrev.diff/
webrev :
https://cr.opensolaris.org/action/browse/caiman/nirmal27/7108281-rev1/webrev/
Slim-test results :
/net/indiana-build.us.oracle.com/export/home/na210770/ai/rbac/slim-test
Detailed Manual tests :
/net/indiana-build.us.oracle.com/export/home/na210770/ai/rbac/manual-testing
Pep8 is clean.
Ran pylint and removed "unused imports".
Test cases : I will file a separate CR and write test case for
functions/class introduced.
Let me know if I need to run other tests.
Thanks
Nirmal
On 04/12/12 04:24, Sue Sohn wrote:
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
