I made a comment regarding the TableNotFoundException hierarchy at [0]. I don't understand why our specific exceptions (TNFE for example) don't extend AccumuloException. If the hierarchy is changed in our Exception classes, then the signatures of the other methods don't need to be changed. Calling code that catches AccumuloException will still work. Calling code that catches AccumuloException and checks the cause for TNFE won't work unless we do something like call `super.initCause(this)` after initializing the parent Exception.
[0] https://github.com/apache/accumulo/pull/5595#issuecomment-2922377649 On Tue, Jun 10, 2025 at 3:53 PM Dominic Garguilo <domgargu...@apache.org> wrote: > Hi all, > > I'd like to discuss how TableNotFoundException (TNFE) is handled in > some of our TableOperations methods. In several places, TNFE is > neither declared nor thrown; instead it’s wrapped in an > AccumuloException (AE). That pushes the burden onto calling code to > discover the wrapping (by reading Javadoc or source) and then to > handle it—both error-prone. > > Obviously, the "right" solution is to have all applicable methods > declare and throw TNFE directly. The blocker is that it’s a public-API > change. This came up in the context of Accumulo PR #5540 [1], which > refactors some TableOperationsImpl methods to use a single > modifyProperties() call instead of multiple > setProperty()/removeProperty() calls for atomicity. Since > modifyProperties() doesn’t throw TNFE (but wraps it), I implemented a > hacky workaround to unwrap and re-throw TNFE without changing the API. > We should replace that workaround with a more permanent solution, so > I’m opening this thread to discuss possible paths forward. > > Possible paths forward: > > 1. Backport the workaround into 2.1 > Continue using the internal "unwrap AE -> TNFE" helper. Preserves > the API and atomic benefits of modifyProperties(), but perpetuates the > hacky code. > > 2. Break SemVer in 2.1 > Change existing signatures so that modifyProperties(), > setProperty(), etc., declare and throw TNFE directly. Clean and > immediate, but violates SemVer and could break subclasses. > > 3. Deprecate & add replacements in 2.1 > Mark the old methods @Deprecated and introduce new ones (e.g. > mutateProperties()) with correct throws. Signals intent clearly, but > adds API churn in a minor release. > > 4. Postpone public-API fixes until 3.1/4.0 > - In 2.1: use the unwrap helper internally where needed. > - In 3.1: deprecate the old methods and introduce properly-declared > replacements. > - In 4.0: remove or adjust the deprecated methods entirely. > > Any and all suggestions or feedback are welcome! > > [1] https://github.com/apache/accumulo/pull/5540 >
