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

Ade Lee alee at redhat.com
Tue Feb 14 04:12:18 UTC 2012


acked by endi.

Added some additional details in readme and pushed to master.

On Mon, 2012-02-13 at 17:03 -0500, Ade Lee wrote:
> 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel





More information about the Pki-devel mailing list