[Pki-devel] [PATCH] patches 0015 and 0016 - cleanup DRM client and add python client code

Endi Sukma Dewata edewata at redhat.com
Sat Feb 11 01:03:52 UTC 2012


On 2/9/2012 2:30 PM, Ade Lee wrote:
> Please review.
>
> This applies on top of jmagne's patches.  As jmagne makes changes, these
> may need to be rebased.  But this is here to get the review going.
>
> Thanks, Ade

Some comments, most of them have been discussed already:

1. There are some formatting issues due to tabs.

2. Some RPM packages are needed in order to run the test properly, they 
should be documented or specified in the BuildRequires.

3. The test parameters in drmclient are hardcoded (e.g. path, file name, 
cert nickname, port). There should be a way to configure it.

4. The testing procedure should be documented.

5. The drmclient uses hardcoded path /kra:

    self._request('/kra/pki/keyrequest/archive', ...)

It's not needed now, but suppose we want to support customizable 
subsystem name we should make it configurable, for example:

    self._request(kra_url, '/pki/keyrequest/archive', ...)

6. We should use unit testing framework for both Java & Python tests.

7. Is there a way to clean up the test data from the server so they do 
not accumulate?

8. In GeneratePKIArchiveOptions read() and write() the nested try-block 
can be flattened by moving the inner finally-clause after the outer 
catch-clause.

-- 
Endi S. Dewata




More information about the Pki-devel mailing list