On 09/06/2016 04:49 PM, Fraser Tweedale wrote:
On Tue, Aug 30, 2016 at 10:23:10AM +0200, Martin Babinsky wrote:
On 08/30/2016 10:09 AM, Jan Cholasta wrote:
Hi,

On 30.8.2016 09:56, Martin Babinsky wrote:
On 08/25/2016 10:25 AM, Fraser Tweedale wrote:
Hi team,

The attached patch fixes
https://fedorahosted.org/freeipa/ticket/6257.

The behaviour of cert-request when the CA is disabled is not very
nice (it reports a server error from Dogtag).  The Dogtag REST
interface gives much better errors so I plan to move to it in a
later change (which will also address
https://fedorahosted.org/freeipa/ticket/3473, in part).

Thanks,
Fraser




HI Fraser,

I have a couple of comments below:

1.)
@@ -25,6 +33,10 @@ EXAMPLES:
     ipa ca-add puppet --desc "Puppet" \\
         --subject "CN=Puppet CA,O=EXAMPLE.COM"

+  Disable a CA.
+
+    ipa ca-disable puppet
+
 """)

You missed an example of `ca-enable` command in the doc string.

2.)

Regarding implementation of ca_enable/disable, I think you can reduce
the amount of code duplication by employing a base class which will look
up the required sub-CA and call the RA backend method required by the
subclass. See the attached untested diff (passes lint) for details.

Looks like I forgot how to OOP while on PTO :) Honza is right, of course,
see the example code in the attached diff (again not tested, just a quick
example).

Updated patch attached, implemented inheritance suggestion and
expanding plugin help.

Thanks,
Fraser


Thanks, ACK.

Pushed to:
master: c7e0dbc4e174d0bb7577de18cdb2f414f4199c57
ipa-4-4: b037e54e457d731cd16144df7573f4c85d79368a

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to