[Pki-devel] [PATCH] refactor installation code to use python client instead of jython

[Ade Lee]

Attached new patch to be applied on top of 122, 123.

The patch also addresses some of the issues in the previous email.
See comments below.

On Wed, 2013-03-20 at 16:57 -0500, Endi Sukma Dewata wrote:
> On 3/20/2013 10:19 AM, Endi Sukma Dewata wrote:
> > On 3/19/2013 3:54 PM, Ade Lee wrote:
> >> This is a pretty big change, but we want to get it into 10.0.2 so that
> >> we can eliminate our dependency on jython.
> >>
> >> So far, its been tested against a straight CA install.  I plan to
> >> continue testing against other configurations, but as the code change is
> >> quite large, I want to start the review early.
> 
> Additional comments for patch #122:
> 
> 7. All command line arguments for should be quoted in case they contain 
> spaces. This including paths and file names.
> 
> It would be better to use subprocess.call() instead of os.system(). The 
> arguments can be specified as a list, so they don't need to be quoted.
> 
Done for code added in this patch.
There are other parts of pkispawn that need to be checked for this
issue.

> 8. In general a function should not call sys.exit() on error. It would 
> be better to raise an Error and let the main program handle it.
> 
This is a general problem in pkispawn (not just in the code submitted).
In keeping with the behavior in pkijython, this was continued in the
code submitted.  A ticket should be opened to address this throughout
the scriptlets.

> 9. Boolean attributes (e.g. isClone, backupKeys, importAdminCert) in 
> Python objects should use real boolean values instead of string.
> 
There is already a ticket to make the fields in the java object
ConfigurationRequest use real boolean values, slated for 10.1.  Python
clients should be handled then too.

> 10. Currently the config_client is instantiated as a global variable in 
> pkihelper.py which may limit its reusability. It would be better to 
> instantiate it where it's actually used (in configuration.py).
> 
Done.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pki-vakwetu-0124-Address-various-issues-from-review.patch
Type: text/x-patch
Size: 18085 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20130321/8a8ff555/attachment.bin>