Skip to content

Commit

Permalink
Merge pull request #13701 from soltysh/request_timeout
Browse files Browse the repository at this point in the history
Automatic merge from submit-queue (batch tested with PRs 18503, 18399, 13701, 18513, 18515).

UPSTREAM: 51042: Allow passing request-timeout from NewRequest all the way down

This was split from openshift/origin#13458.

@smarterclayton let's continue the discussion here.

Origin-commit: 0fd7db928c80fe528c51848eebb85b9544beb72f


Kubernetes-commit: c6df867ec232478651d96abce6d459be747defed
  • Loading branch information
k8s-publishing-bot committed Feb 9, 2018
2 parents 859b7fe + d46dd85 commit 6187e61
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 11 deletions.
4 changes: 2 additions & 2 deletions rest/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,9 @@ func (c *RESTClient) Verb(verb string) *Request {
backoff := c.createBackoffMgr()

if c.Client == nil {
return NewRequest(nil, verb, c.base, c.versionedAPIPath, c.contentConfig, c.serializers, backoff, c.Throttle)
return NewRequest(nil, verb, c.base, c.versionedAPIPath, c.contentConfig, c.serializers, backoff, c.Throttle, 0)
}
return NewRequest(c.Client, verb, c.base, c.versionedAPIPath, c.contentConfig, c.serializers, backoff, c.Throttle)
return NewRequest(c.Client, verb, c.base, c.versionedAPIPath, c.contentConfig, c.serializers, backoff, c.Throttle, c.Client.Timeout)
}

// Post begins a POST request. Short for c.Verb("POST").
Expand Down
2 changes: 1 addition & 1 deletion rest/fake/fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (c *RESTClient) request(verb string) *restclient.Request {
serializers.StreamingSerializer = info.StreamSerializer.Serializer
serializers.Framer = info.StreamSerializer.Framer
}
return restclient.NewRequest(c, verb, &url.URL{Host: "localhost"}, c.VersionedAPIPath, config, serializers, nil, nil)
return restclient.NewRequest(c, verb, &url.URL{Host: "localhost"}, c.VersionedAPIPath, config, serializers, nil, nil, 0)
}

func (c *RESTClient) Do(req *http.Request) (*http.Response, error) {
Expand Down
3 changes: 2 additions & 1 deletion rest/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ type Request struct {
}

// NewRequest creates a new request helper object for accessing runtime.Objects on a server.
func NewRequest(client HTTPClient, verb string, baseURL *url.URL, versionedAPIPath string, content ContentConfig, serializers Serializers, backoff BackoffManager, throttle flowcontrol.RateLimiter) *Request {
func NewRequest(client HTTPClient, verb string, baseURL *url.URL, versionedAPIPath string, content ContentConfig, serializers Serializers, backoff BackoffManager, throttle flowcontrol.RateLimiter, timeout time.Duration) *Request {
if backoff == nil {
glog.V(2).Infof("Not implementing request backoff strategy.")
backoff = &NoBackoff{}
Expand All @@ -131,6 +131,7 @@ func NewRequest(client HTTPClient, verb string, baseURL *url.URL, versionedAPIPa
serializers: serializers,
backoffMgr: backoff,
throttle: throttle,
timeout: timeout,
}
switch {
case len(content.AcceptContentTypes) > 0:
Expand Down
14 changes: 7 additions & 7 deletions rest/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@ import (
)

func TestNewRequestSetsAccept(t *testing.T) {
r := NewRequest(nil, "get", &url.URL{Path: "/path/"}, "", ContentConfig{}, Serializers{}, nil, nil)
r := NewRequest(nil, "get", &url.URL{Path: "/path/"}, "", ContentConfig{}, Serializers{}, nil, nil, 0)
if r.headers.Get("Accept") != "" {
t.Errorf("unexpected headers: %#v", r.headers)
}
r = NewRequest(nil, "get", &url.URL{Path: "/path/"}, "", ContentConfig{ContentType: "application/other"}, Serializers{}, nil, nil)
r = NewRequest(nil, "get", &url.URL{Path: "/path/"}, "", ContentConfig{ContentType: "application/other"}, Serializers{}, nil, nil, 0)
if r.headers.Get("Accept") != "application/other, */*" {
t.Errorf("unexpected headers: %#v", r.headers)
}
Expand All @@ -86,7 +86,7 @@ func TestRequestSetsHeaders(t *testing.T) {
config := defaultContentConfig()
config.ContentType = "application/other"
serializers := defaultSerializers(t)
r := NewRequest(server, "get", &url.URL{Path: "/path"}, "", config, serializers, nil, nil)
r := NewRequest(server, "get", &url.URL{Path: "/path"}, "", config, serializers, nil, nil, 0)

// Check if all "issue" methods are setting headers.
_ = r.Do()
Expand Down Expand Up @@ -341,7 +341,7 @@ func TestResultIntoWithNoBodyReturnsErr(t *testing.T) {

func TestURLTemplate(t *testing.T) {
uri, _ := url.Parse("http://localhost")
r := NewRequest(nil, "POST", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil)
r := NewRequest(nil, "POST", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0)
r.Prefix("pre1").Resource("r1").Namespace("ns").Name("nm").Param("p0", "v0")
full := r.URL()
if full.String() != "http://localhost/pre1/namespaces/ns/r1/nm?p0=v0" {
Expand Down Expand Up @@ -403,7 +403,7 @@ func TestTransformResponse(t *testing.T) {
{Response: &http.Response{StatusCode: 200, Body: ioutil.NopCloser(bytes.NewReader(invalid))}, Data: invalid},
}
for i, test := range testCases {
r := NewRequest(nil, "", uri, "", defaultContentConfig(), defaultSerializers(t), nil, nil)
r := NewRequest(nil, "", uri, "", defaultContentConfig(), defaultSerializers(t), nil, nil, 0)
if test.Response.Body == nil {
test.Response.Body = ioutil.NopCloser(bytes.NewReader([]byte{}))
}
Expand Down Expand Up @@ -554,7 +554,7 @@ func TestTransformResponseNegotiate(t *testing.T) {
serializers.RenegotiatedDecoder = negotiator.invoke
contentConfig := defaultContentConfig()
contentConfig.ContentType = test.ContentType
r := NewRequest(nil, "", uri, "", contentConfig, serializers, nil, nil)
r := NewRequest(nil, "", uri, "", contentConfig, serializers, nil, nil, 0)
if test.Response.Body == nil {
test.Response.Body = ioutil.NopCloser(bytes.NewReader([]byte{}))
}
Expand Down Expand Up @@ -1480,7 +1480,7 @@ func TestAbsPath(t *testing.T) {
{"/p1/api/p2", "/api/r1", "/api/", "/p1/api/p2/api/"},
} {
u, _ := url.Parse("http://localhost:123" + tc.configPrefix)
r := NewRequest(nil, "POST", u, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil).Prefix(tc.resourcePrefix).AbsPath(tc.absPath)
r := NewRequest(nil, "POST", u, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0).Prefix(tc.resourcePrefix).AbsPath(tc.absPath)
if r.pathPrefix != tc.wantsAbsPath {
t.Errorf("test case %d failed, unexpected path: %q, expected %q", i, r.pathPrefix, tc.wantsAbsPath)
}
Expand Down

0 comments on commit 6187e61

Please sign in to comment.