Backport Heartbeat, MaxRetries and MaxRetryWaitTime #20
Conversation
bmeike
force-pushed
the
android/release/hydrogen
branch
from
ff74ed8
to
7f68df0
Compare
bmeike
force-pushed
the
candidate/2.8.4
branch
from
a9046ea
to
2f37f23
Compare
There was a problem hiding this comment.
I feel badly for saying "you did too much" but to keep us aligned we should only be putting in heartbeat (and not the other two related to max retry) since .NET and iOS are not scheduled to have it for 2.8.x. The logic can stay, but is there a way to make it not visible externally?
bmeike
force-pushed
the
candidate/2.8.4
branch
from
a5a840f
to
2e7d9d5
Compare
bmeike
force-pushed
the
candidate/2.8.4
branch
from
2e7d9d5
to
fbb99ec
Compare
|
Oh good grief. Ya, sure. Will fix. |
| ? maxRetries | ||
| : ((continuous) | ||
| ? AbstractCBLWebSocket.DEFAULT_ONE_SHOT_MAX_RETRIES | ||
| : AbstractCBLWebSocket.DEFAULT_CONTINUOUS_MAX_RETRIES); |
There was a problem hiding this comment.
@pasin Should these remain what they are in 2.8.x until 3.0? From what I can tell these are the new defaults.
There was a problem hiding this comment.
I think the changes will be done in 3.0, otherwise java/android will have different behavior from the other platforms.
There was a problem hiding this comment.
This function is not visible in 2.8.4 (x == 4, unless I am mistaken). Whether these are the correct values seems a question only for 3.0.0, I think.
Unless you want me to actually remove them.
The amount of time this simple backport has taken is pretty staggering. It could have been a simple cherry-pick. I wish we could find a more efficient way of doing this kind of stuff...
There was a problem hiding this comment.
I might not understand this correctly. It seems like maxRetries is default to -1 so it means that getMaxRetries() will either use DEFAULT_ONE_SHOT_MAX_RETRIES or DEFAULT_CONTINUOUS_MAX_RETRIES?
Update: getMaxRetries() is used internally in effectiveOptions() method.
There was a problem hiding this comment.
Please tell me what you want, so that I can do it. If I missed something in the spec, my apologies. Just point me at it.
There was a problem hiding this comment.
DEFAULT_ONE_SHOT_MAX_RETRIES or DEFAULT_CONTINUOUS_MAX_RETRIES are for Lithium. It's OK to keep getMaxRetries() to use in Lithium but can you remove or comment out line 452 and 453 that use the value?
|
Removed all traces of maxretries and -interval |
| @@ -144,7 +140,7 @@ public final ReplicatorConfiguration setChannels(@Nullable List<String> channels | |||
| /** | |||
| * Sets the the conflict resolver. | |||
| * | |||
| * @param conflictResolver A conflict resolver. | |||
| * @param conflictResolver The replicator type. | |||
There was a problem hiding this comment.
Wrong description. Can you revert this back to A conflict resolver?
There was a problem hiding this comment.
Not a regression. That comment has been there for ages. WIll fix in 3.0
Backport Heartbeat, MaxRetries and MaxRetryWaitTime, now part of 2.8.4