Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Infinite Socket Timeout can be specified #2028

Merged
merged 5 commits into from
Dec 8, 2020
Merged

Infinite Socket Timeout can be specified #2028

merged 5 commits into from
Dec 8, 2020

Conversation

sazzad16
Copy link
Collaborator

@sazzad16 sazzad16 commented Jul 20, 2019 •
edited
Loading

Resolves #426
Resolves #1537
Resolves #2027
Closes #2207
Resolves #2217

@sazzad16 sazzad16 added this to the 3.2.0 milestone Jul 20, 2019
@sazzad16 sazzad16 requested a review from gkorland July 27, 2019 08:42
@gkorland
Copy link
Contributor

@sazzad16 shouldn't just overloading public void setTimeoutInfinite() with timeout parameter will be better? public void setTimeoutInfinite(long timeout)

@sazzad16
Copy link
Collaborator Author

@gkorland How would we get the timeout to call setTimeoutInfinite(timeout)? I can see two other options and I prefer current implementation over both of those.

  • Repeat all the methods that calls setTimeoutInfinite() with a new timeout parameter.

  • Hold timeout in a variable in BinaryJedis. Still we'll need all those constructors. And (Binary)Jedis hardly holds any parameter.

@sazzad16 sazzad16 modified the milestones: 3.2.0, 3.3.0 Nov 25, 2019
@rominzxx
Copy link

rominzxx commented Jan 15, 2020 •
edited
Loading

Hey, can someone review/merge this? It's an important fix.

rominzxx
rominzxx previously approved these changes Jan 15, 2020
View reviewed changes
@tokenmathguy
Copy link

+1 to the fix @sazzad16 @gkorland

It looks like there are some merge conflicts. Anything the community can assist with?

@sazzad16 sazzad16 modified the milestones: 3.3.0, 3.4.0 Apr 26, 2020
@mkozioro
Copy link

mkozioro commented Jun 4, 2020

Hi @sazzad16
Issue that is fixed by this PR is sometimes still occurring in our system. Do you think it will be merged some day?

sandrzejczak added a commit to sandrzejczak/jedis that referenced this pull request Jun 9, 2020
@sandrzejczak
Fixed merge conflicts in "Infinite Socket Timeout can be specified re…
fc1dadb 
…dis#2028" PR.
@sandrzejczak sandrzejczak mentioned this pull request Jun 9, 2020
Closed
@sazzad16 sazzad16 force-pushed the timeout-infinite branch 2 times, most recently from 4976e26 to c97c541 Compare November 25, 2020 07:54
@sazzad16
Copy link
Collaborator Author

sazzad16 commented Dec 3, 2020

@gkorland PING

@sazzad16 sazzad16 merged commit 1e1ce47 into redis:master Dec 8, 2020
@mkozioro
Copy link

mkozioro commented Dec 8, 2020

Oh my god. After so much time finally merged. It's a great day. :)

@sazzad16 sazzad16 mentioned this pull request Dec 14, 2020
Closed
@sazzad16 sazzad16 deleted the timeout-infinite branch December 26, 2020 14:24
@dependabot dependabot bot mentioned this pull request Mar 16, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gkorland gkorland gkorland approved these changes

@rominzxx rominzxx rominzxx left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.4.0
5 participants
@sazzad16 @gkorland @rominzxx @tokenmathguy @mkozioro