-
Notifications
You must be signed in to change notification settings - Fork 210
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.
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
Set idleTimeout for Jetty http client #1006
Conversation
144153a
to
d1bdab4
Compare
@@ -15,6 +15,7 @@ | |||
private static final int DEFAULT_MESSAGE_TTL = 3600; | |||
private static final int DEFAULT_MESSAGE_BACKOFF = 100; | |||
private static final int DEFAULT_REQUEST_TIMEOUT = 1000; | |||
private static final int DEFAULT_SOCKET_TIMEOUT = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you set it to 0 it will be no socket timeout at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So currently created subscriptions are unaffected. With this default value, only subscription's request timeouts will be respected
// given | ||
HttpRequestFactory httpRequestFactory = new HttpRequestFactory(client, | ||
100, | ||
100,1000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
100,1000, | |
100, 1000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intellij's arguments hints :D
import com.fasterxml.jackson.annotation.JsonProperty; | ||
import java.util.Optional; | ||
|
||
public class OAuthProviderWithOptionalSocketTimeout { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we really need this class (a class with such explicit name)? :) I mean, can't we just add the optional socket timeout to the existing one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class is only needed to be able to fetch existing zookeeper saved oauth providers. Its implementation detail. BTW this class should be placed in infrastructure package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
This PR fixes #1004