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

New patches attached. See comments below: On Fri, 2012-02-10 at 19:03 -0600, Endi Sukma Dewata wrote: > 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. > fixed > 2. Some RPM packages are needed in order to run the test properly, they > should be documented or specified in the BuildRequires. > documented - see readme. > 3. The test parameters in drmclient are hardcoded (e.g. path, file name, > cert nickname, port). There should be a way to configure it. > command line params added > 4. The testing procedure should be documented. > see readme. > 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', ...) > lets defer this for when we want to do this. I suspect it may never happen. > 6. We should use unit testing framework for both Java & Python tests. > Yes - we have a trac task to junitize this work. > 7. Is there a way to clean up the test data from the server so they do > not accumulate? > We can look at that when we do the junit work. In general, this is not easy. > 8. In GeneratePKIArchiveOptions read() and write() the nested try-block > can be flattened by moving the inner finally-clause after the outer > catch-clause. > done. _______________________________________________ Pki-devel mailing list Pki-devel(a)redhat.com https://www.redhat.com/mailman/listinfo/pki-devel