ctubbsii commented on PR #4816: URL: https://github.com/apache/accumulo/pull/4816#issuecomment-2344619347
After a conversation with @kevinrr888, I added commit 5924b02c065297886ede479fbcf9750e699e4362 to incorporate some of the changes we discussed, that he intends to build on and polish up. The main points included were: * Push a reasonable `getConfig()` implementation up into `IteratorEnvironment`'s default method (depends on the ability to have `getTableId()` return a null value) * Include concrete implementations of `getPluginEnv()` instead of just `getServiceEnv()` to make it easier to just drop the deprecated one when merging to newer branches * Consider using a Proxy instance of TableId instead of a real instance, so it isn't possible to be confused with any similar user-constructed TableId (may not work) * Update javadocs in RFile to make it clearer how properties set on the scanner builder (such as iterators) will be available to client-side iterators from the plugin environment The main thing left unresolved after our conversation was how to fix the mismatch between the implementations and the javadoc for all the methods that inconsistently return null or throw IllegalStateException or UnsupportedOperationException, when calling a method that doesn't apply to a given situation (like asking for a table ID or table name when operating directly on RFiles or asking if it's a user-initiated compaction when it's a scan). Nearly every caller seems to have a unique set a requirements/implementation, and it's a bit hard to home in on the best implementation, especially for the default methods, and make sure the javadoc matches the behavior. -- 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]
