milleruntime commented on PR #2946: URL: https://github.com/apache/accumulo/pull/2946#issuecomment-1255231894
> Do you think this should not be merged then and leave CryptoException as a RuntimeException. I was thinking that having a known checked Exception in the SPI methods makes sense so that errors specific to crypto can be handled. For most situations, I don't think there will be much we can do if we catch a CryptoException (coming from a TabletServer, for example) other than restart the server. Since these interfaces are in the SPI, the "client" who handles the exception will usually be server code. And with the way the CryptoService runs within the tserver, there is a good chance a restart will have to happen to reload the service. Eventually, once the Crypto is properly tested we could drop the experimental tag and be able to determine a better way of handling the errors. Or if we create a designated Server running just for Crypto, there may be better ways to handle the errors. The RFile API may be a good place to throw a checked exception for example, allowing the client to handle encryption errors. I do think there is room for improving the overall error handling though. But maybe changing the exception type isn't the right approach. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
