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

Ade Lee alee at redhat.com
Thu Aug 14 02:21:24 UTC 2014


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 at redhat.com
> > > https://www.redhat.com/mailman/listinfo/pki-devel
> > 
> > 
> 





More information about the Pki-devel mailing list