Reconnection support #12
Conversation
peiyush13
commented
Jul 5, 2018
peiyush13
commented
- Added Reconnection Support as per the issue :- Reconnection support :) #9
maanavshah
force-pushed
the
reconnection_support
branch
from
b52baee to
446500a
Compare
peiyush13
force-pushed
the
reconnection_support
branch
2 times, most recently
from
b52baee to
446500a
Compare
maanavshah
force-pushed
the
reconnection_support
branch
2 times, most recently
from
e82357c to
8018096
Compare
peiyush13
force-pushed
the
reconnection_support
branch
from
8018096 to
0022ddb
Compare
Added reconnection strategy for socketclusterclient
peiyush13
force-pushed
the
reconnection_support
branch
from
0022ddb to
5ba021d
Compare
| include DataModels | ||
| include Reconnect | ||
|
|
||
| attr_accessor :reconnect_interval, :max_reconnect_interval, :reconnect_decay, :max_attempts, :attempts_made |
There was a problem hiding this comment.
Don't need to add attr_accessor for variables used in the class itself and unless we are using strong params.
There was a problem hiding this comment.
- attr_accessors are used here to avoid unnecessary for getter-setter methods, strong params are not applicable here
README.md
Outdated
| ``` | ||
|
|
||
| - For disabling reconnection to server | ||
| - To disable Automatic Reconnection |
There was a problem hiding this comment.
Please change it to: To disable automatic reconnection to server
lib/socketclusterclient/reconnect.rb
Outdated
| def reconnection_attempts_finished | ||
| return @attempts_made == @max_attempts | ||
| end | ||
|
|
There was a problem hiding this comment.
Remove this line, or else run rubocop --autocorrect which will fix this issue.
| # | ||
| # | ||
| def set_reconnection_listener(reconnect_interval, max_reconnect_interval, reconnect_decay, max_attempts) | ||
| @max_reconnect_interval = max_reconnect_interval |
There was a problem hiding this comment.
Please put this line below as it changes the code output. Refer: https://github.com/sacOO7/socketcluster-client-java/blob/master/src/main/java/io/github/sac/ReconnectStrategy.java
There was a problem hiding this comment.
It is not needed as the logical bug here but for the sake of micro optimisation, I am changing it.
lib/socketclusterclient/reconnect.rb
Outdated
| # | ||
| # Module Reconnect provides Reconnection Support | ||
| # | ||
| # @author Piyush Wani <piyush.wani@amuratech.com> |
There was a problem hiding this comment.
Change email to your github email_id
| # | ||
| # @author Piyush Wani <piyush.wani@amuratech.com> | ||
| # | ||
| module Reconnect |
There was a problem hiding this comment.
Please run: rubocop --autocorrect
| socket.connect | ||
| socket.set_reconnection(false) # default reconnection is true | ||
| socket.max_attempts = 5 | ||
| socket.reconnect |
There was a problem hiding this comment.
Can we also add an example, if we set_reconnection to true?
There was a problem hiding this comment.
set_reconnction method is for automatic reconnection and one should not manually reconnect if auto_reconnection is enabled. so no need to add an example for that
Changed code as per the Rubocop generated suggestions
