2:30 p.m.
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
7:03 p.m.
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
4:03 p.m.
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.
10:12 p.m.
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
4700
days inactive
4705
days old
3 comments
2 participants
participants (2)
-
Ade Lee -
Endi Sukma Dewata