jkosh44 commented on issue #978: Use CompleteableFuture compose to centralize commit logic URL: https://github.com/apache/fluo/issues/978#issuecomment-357830449 Ok so I created a pull request, #1001. It's not passing every test but it only seems to be failing 3 tests.Those three test are all from `FailureIT` and it's because `TransactionImpl.commitPrimaryColumn(CommitData cd, Stamp commitStamp)` isn't returning false when it's supposed to. I'm pretty sure it's how I implemented exception handling, if a 'CommitException' is thrown it's somehow not propagated back up to 'commitPrimaryColumn'. I'm going to look at it some more but I though maybe a fresh set of eyes would be helpful. In general the way I dealt with the Visible For Testing methods were as follows. I would create a first step as whatever intermediate step that testing method was starting at, fill in the rest of the steps using 'andThen', and then call compose. In the appropriate step classes themselves in 'getMainOp' i would check if 'stopAfterPrimaryCommit' or 'stopAfter'PreCommit' were true and if so I would return false. Then in 'getFailureOp' if 'stopAfterPrimaryCommit' or 'stopAfter'PreCommit' were true I'd just call 'cd.commitObserver.committed();' It's not super clean in that Essentially I'd be saying the operation failed if those test values were true even though technically they didn't fail. I wasn't sure of a cleaner solution but though this was a good first approach.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
