[
https://issues.apache.org/jira/browse/CURATOR-562?focusedWorklogId=399691&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-399691
]
ASF GitHub Bot logged work on CURATOR-562:
------------------------------------------
Author: ASF GitHub Bot
Created on: 07/Mar/20 15:29
Start Date: 07/Mar/20 15:29
Worklog Time Spent: 10m
Work Description: Randgalt commented on pull request #348: CURATOR-562
Remove ConnectionHandlingPolicy
URL: https://github.com/apache/curator/pull/348#discussion_r389262170
##########
File path: curator-client/src/main/java/org/apache/curator/ConnectionState.java
##########
@@ -218,7 +216,7 @@ public String call()
};
int lastNegotiatedSessionTimeoutMs =
getLastNegotiatedSessionTimeoutMs();
int useSessionTimeoutMs = (lastNegotiatedSessionTimeoutMs > 0) ?
lastNegotiatedSessionTimeoutMs : sessionTimeoutMs;
- ConnectionHandlingPolicy.CheckTimeoutsResult result =
connectionHandlingPolicy.checkTimeouts(hasNewConnectionString,
connectionStartMs, useSessionTimeoutMs, connectionTimeoutMs);
+ CheckTimeoutsResult result = checkTimeouts(hasNewConnectionString,
connectionStartMs, useSessionTimeoutMs, connectionTimeoutMs);
Review comment:
To my eye, the separate method doesn't add much and hurts readability. To
the point, `RESET_CONNECTION`, `CONNECTION_TIMEOUT` and `SESSION_TIMEOUT` are
no longer possible. Only `NOP` and `NEW_CONNECTION_STRING` are possible so
`CheckTimeoutsResult` is no longer needed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 399691)
Time Spent: 20m (was: 10m)
> Remove ConnectionHandlingPolicy
> -------------------------------
>
> Key: CURATOR-562
> URL: https://issues.apache.org/jira/browse/CURATOR-562
> Project: Apache Curator
> Issue Type: Improvement
> Components: Framework
> Reporter: Zili Chen
> Priority: Major
> Fix For: 5.0.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Back to the day we bump Curator from 2.x to 3.0 we introduce
> {{ConnectionHandlingPolicy}} as a bridge that provides different strategy on
> handling different session timeout logic.
> Things changed and now only the {{StandardConnectionHandlingPolicy}} survive,
> who has two methods
> 1. {{checkTimeouts}} totally static utility. Besides, some values in
> {{CheckTimeoutsResult}} seems out of day that we might have a follow-up to
> handle it. Anyway, I don't thing anyone outside Curator should change the
> behavior here since it is tight couple with how {{o.a.c.ConnectionState}}
> works.
> 2. {{callWithRetry}} it is actually a consignee of
> {{RetryLoop#callWithRetry}}. According to its implementation it seems hardly
> an outside user can properly define his {{callWithRetry}} method.
> Thus, I propose that we flatten this legacy class.
> DISCLAIMER:
> The place from where I come here is CURATOR-544 that I'd like to implement a
> user-friendly option to enable Curator blocks its regenerate ZooKeeper client
> logic on {{SESSIONEXPIRED}}.
> However, neither {{ConnectionHandlingPolicy}} nor {{RetryLoop}} nor
> {{RetryPolicy}} is a good place since implementations coupled quite a bit.
> And the real regeneration logic hided inside {{ConnectionState}}.
> Thus I'm willing to do a bit code cleanups to see where we can properly
> inject such logics.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
