kezhuw commented on code in PR #464:
URL: https://github.com/apache/curator/pull/464#discussion_r1212550593
##########
curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java:
##########
@@ -694,17 +714,46 @@ <DATA_TYPE> void
processBackgroundOperation(OperationAndData<DATA_TYPE> operatio
}
}
+ private void closeOperation(OperationAndData<?> operation) {
+ if (operation.getCallback() == null) {
+ return;
+ }
+ CuratorEvent event = new CuratorEventImpl(this,
CuratorEventType.CLOSING, KeeperException.Code.SESSIONEXPIRED.intValue(), null,
null, operation.getContext(), null, null, null, null, null, null);
Review Comment:
Yeh, it is hard to choose 😮💨. `performBackgroundOperation` uses
`CONNECTIONLOSS` for callbacks when retry gave up in case of no active
connection. For `BackgroundCallback`, I think it is more appropriate to treat
`CuratorEvent::getResultCode` as authority for success or not and. From client
side, no updates are required (at my best guess).
Aside from this, `CuratorEvent::getType` is simple a "nice to match" for
`BackgroundCallback`. It plays a significant role to differentiate events in
`CuratorListener` which is catch-all fallback for events with no
`BackgroundCallback`.
https://github.com/apache/curator/blob/328f985304b07cec046e4b9e4244a9c962fc91f5/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java#L681-L687
`WATCHED` is used in `CONNECTIONLOSS` case already. I would prefer to let
`OperationAndData` carrying `CuratorEventType` from creation so we could
delivery matching event type. In case of closing, `CuratorEventType.CLOSING`
is not a bad choice(actually there is no good candidate currently), but it is
not worth us to treat `CuratorEventType.CLOSING` specially for
`BackgroundCallback`. Maybe a separated issue to cover this ?
https://github.com/apache/curator/blob/328f985304b07cec046e4b9e4244a9c962fc91f5/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java#L1019-L1024
--
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]
