[CONTROLLER-624] Usability enhancement to WriteTransaction commit method Created: 14/Jul/14 Updated: 22/Jul/14 Resolved: 22/Jul/14 |
|
| Status: | Resolved |
| Project: | controller |
| Component/s: | mdsal |
| Affects Version/s: | Helium |
| Fix Version/s: | None |
| Type: | Improvement | ||
| Reporter: | Tom Pantelis | Assignee: | Tom Pantelis |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Environment: |
Operating System: All |
||
| Description |
|
The current WrteTransaction commit method returns RpcResult<TransactionStatus>: ListenableFuture<RpcResult<TransactionStatus>> commit(); Typical client usage is to use a Futures.addCallback: Futures.addCallback( commitFuture, else { // failed }} @Override } ); Notice that the client has to handle failure conditions in 2 places and in 2 different ways (by checking TransactionStatus and in onFailure). Also the onSuccess method may be a bit misleading as it might not have been successful depending on the TransactionStatus. Also at this point the TransactionStatus could only be COMMITED or FAILURE which is really redundant with RpcResult#isSuccessful. I propose changing the commit method to: CheckedFuture<Void,TransactionCommitFailedException> ready(); This method removes the RpcResult entirely and all errors conditions are conveyed via one path, via TransactionCommitFailedException. Nothing need be returned for a successful commit. The RpcResult allows for rich error info to be conveyed via RpcError. We can add RpcErrors to TransactionCommitFailedException for clients that are interested (eg restconf). The CheckedFuture explicitly conveys that clients can expect only TransactionCommitFailedExceptions. Client code then becomes: Futures.addCallback( commitFuture, new FutureCallback<Void>() { @Override } ); which is cleaner. |
| Comments |
| Comment by Tom Pantelis [ 16/Jul/14 ] |