Please change the message in the ConnectorNotFoundException() from "KRAConnector does not exist" to "No KRAConnector has been configured". Otherwise, ACK. No need for re-review with this change. Ade On Tue, 2014-08-12 at 09:42 -0400, Abhishek Koneru wrote: > Please review the patch with the comments addressed. > > -- Abhishek > On Thu, 2014-08-07 at 22:35 -0400, Ade Lee wrote: > > 1. You add the method GET /admin/kraconnector/connectorInfo. The last > > bit is unnecessary -- Just use GET /admin/kraconnector. > > > > 2. There are a number of places with formatting issues concerning > > parenthesis. As an example in KRAConnectorShowCLI.java, > > > > if(host.indexOf(' ') == -1){ > > host += ":"+info.getPort(); > > }else{ > > ... > > > > Change to : > > > > if (host.indexOf(' ') == -1) { > > host += ":"+info.getPort(); > > } else { > > ... > > Check for other similar cases. > > > > 3. In KRAConnectorProcessor getConnectorInfo() , you throw an EBaseException > > if the connector does not exist. This will translate into a 500 error. > > > > You should instead throw some kind of Resource Not Found exception, so that we > > return a 404. > > > > Ade > > > > On Tue, 2014-08-05 at 14:24 -0400, Abhishek Koneru wrote: > > > Please review the patch that adds a new CLI command pki > > > ca-kra-connector-show. > > > It prints the details of the kra connector registered with the CA. > > > > > > -- Abhishek > > > _______________________________________________ > > > Pki-devel mailing list > > > Pki-devel(a)redhat.com > > > https://www.redhat.com/mailman/listinfo/pki-devel > > > > >