Re: [Pki-devel] [PATCH] 102 Add an interface for kraconnector-show (#479)

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 > >