[arvados] updated: 2.6.0-206-geca324141
git repository hosting
git at public.arvados.org
Mon May 29 19:14:03 UTC 2023
Summary of changes:
sdk/go/arvadosclient/arvadosclient.go | 99 ++++++++++--------------------
sdk/go/arvadosclient/arvadosclient_test.go | 50 +++++++++------
2 files changed, 62 insertions(+), 87 deletions(-)
via eca324141778eb082871f5f8bd6b398edc6ac92a (commit)
from 32a45cde3bd2ef806032d237b38b4d022774a2c2 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit eca324141778eb082871f5f8bd6b398edc6ac92a
Author: Tom Clegg <tom at curii.com>
Date: Mon May 29 15:11:27 2023 -0400
20540: Use arvados.Client for arvadosclient.ArvadosClient reqs.
Caller-specified Retries is used as a timeout in minutes.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go
index 13b3a30ac..516187c0e 100644
--- a/sdk/go/arvadosclient/arvadosclient.go
+++ b/sdk/go/arvadosclient/arvadosclient.go
@@ -231,74 +231,37 @@ func (c *ArvadosClient) CallRaw(method string, resourceType string, uuid string,
vals.Set(k, string(m))
}
}
-
- retryable := false
- switch method {
- case "GET", "HEAD", "PUT", "OPTIONS", "DELETE":
- retryable = true
- }
-
- // Non-retryable methods such as POST are not safe to retry automatically,
- // so we minimize such failures by always using a new or recently active socket
- if !retryable {
- if time.Since(c.lastClosedIdlesAt) > MaxIdleConnectionDuration {
- c.lastClosedIdlesAt = time.Now()
- c.Client.Transport.(*http.Transport).CloseIdleConnections()
- }
- }
-
- // Make the request
var req *http.Request
- var resp *http.Response
-
- for attempt := 0; attempt <= c.Retries; attempt++ {
- if method == "GET" || method == "HEAD" {
- u.RawQuery = vals.Encode()
- if req, err = http.NewRequest(method, u.String(), nil); err != nil {
- return nil, err
- }
- } else {
- if req, err = http.NewRequest(method, u.String(), bytes.NewBufferString(vals.Encode())); err != nil {
- return nil, err
- }
- req.Header.Add("Content-Type", "application/x-www-form-urlencoded")
- }
-
- // Add api token header
- req.Header.Add("Authorization", fmt.Sprintf("OAuth2 %s", c.ApiToken))
- if c.RequestID != "" {
- req.Header.Add("X-Request-Id", c.RequestID)
- }
-
- resp, err = c.Client.Do(req)
- if err != nil {
- if retryable {
- time.Sleep(RetryDelay)
- continue
- } else {
- return nil, err
- }
- }
-
- if resp.StatusCode == http.StatusOK {
- return resp.Body, nil
+ if method == "GET" || method == "HEAD" {
+ u.RawQuery = vals.Encode()
+ if req, err = http.NewRequest(method, u.String(), nil); err != nil {
+ return nil, err
}
-
- defer resp.Body.Close()
-
- switch resp.StatusCode {
- case 408, 409, 422, 423, 500, 502, 503, 504:
- time.Sleep(RetryDelay)
- continue
- default:
- return nil, newAPIServerError(c.ApiServer, resp)
+ } else {
+ if req, err = http.NewRequest(method, u.String(), bytes.NewBufferString(vals.Encode())); err != nil {
+ return nil, err
}
+ req.Header.Add("Content-Type", "application/x-www-form-urlencoded")
}
-
- if resp != nil {
+ if c.RequestID != "" {
+ req.Header.Add("X-Request-Id", c.RequestID)
+ }
+ client := arvados.Client{
+ Client: c.Client,
+ APIHost: c.ApiServer,
+ AuthToken: c.ApiToken,
+ Insecure: c.ApiInsecure,
+ Timeout: 30 * RetryDelay * time.Duration(c.Retries),
+ }
+ resp, err := client.Do(req)
+ if err != nil {
+ return nil, err
+ }
+ if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusNoContent {
+ defer resp.Body.Close()
return nil, newAPIServerError(c.ApiServer, resp)
}
- return nil, err
+ return resp.Body, nil
}
func newAPIServerError(ServerAddress string, resp *http.Response) APIServerError {
@@ -332,12 +295,12 @@ func newAPIServerError(ServerAddress string, resp *http.Response) APIServerError
// Call an API endpoint and parse the JSON response into an object.
//
-// method - HTTP method: GET, HEAD, PUT, POST, PATCH or DELETE.
-// resourceType - the type of arvados resource to act on (e.g., "collections", "pipeline_instances").
-// uuid - the uuid of the specific item to access. May be empty.
-// action - API method name (e.g., "lock"). This is often empty if implied by method and uuid.
-// parameters - method parameters.
-// output - a map or annotated struct which is a legal target for encoding/json/Decoder.
+// method - HTTP method: GET, HEAD, PUT, POST, PATCH or DELETE.
+// resourceType - the type of arvados resource to act on (e.g., "collections", "pipeline_instances").
+// uuid - the uuid of the specific item to access. May be empty.
+// action - API method name (e.g., "lock"). This is often empty if implied by method and uuid.
+// parameters - method parameters.
+// output - a map or annotated struct which is a legal target for encoding/json/Decoder.
//
// Returns a non-nil error if an error occurs making the API call, the
// API responds with a non-successful HTTP status, or an error occurs
diff --git a/sdk/go/arvadosclient/arvadosclient_test.go b/sdk/go/arvadosclient/arvadosclient_test.go
index 27e23c1ae..b074e21e8 100644
--- a/sdk/go/arvadosclient/arvadosclient_test.go
+++ b/sdk/go/arvadosclient/arvadosclient_test.go
@@ -31,7 +31,7 @@ type ServerRequiredSuite struct{}
func (s *ServerRequiredSuite) SetUpSuite(c *C) {
arvadostest.StartKeep(2, false)
- RetryDelay = 0
+ RetryDelay = 2 * time.Second
}
func (s *ServerRequiredSuite) TearDownSuite(c *C) {
@@ -248,7 +248,7 @@ func (s *UnitSuite) TestPDHMatch(c *C) {
type MockArvadosServerSuite struct{}
func (s *MockArvadosServerSuite) SetUpSuite(c *C) {
- RetryDelay = 0
+ RetryDelay = 100 * time.Millisecond
}
func (s *MockArvadosServerSuite) SetUpTest(c *C) {
@@ -279,15 +279,17 @@ type APIStub struct {
}
func (h *APIStub) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
- if req.URL.Path == "/redirect-loop" {
- http.Redirect(resp, req, "/redirect-loop", http.StatusFound)
- return
- }
- if h.respStatus[h.retryAttempts] < 0 {
- // Fail the client's Do() by starting a redirect loop
- http.Redirect(resp, req, "/redirect-loop", http.StatusFound)
+ if status := h.respStatus[h.retryAttempts]; status < 0 {
+ // Fail the client's Do() by hanging up without
+ // sending an HTTP response header.
+ conn, _, err := resp.(http.Hijacker).Hijack()
+ if err != nil {
+ panic(err)
+ }
+ conn.Write([]byte("zzzzzzzzzz"))
+ conn.Close()
} else {
- resp.WriteHeader(h.respStatus[h.retryAttempts])
+ resp.WriteHeader(status)
resp.Write([]byte(h.responseBody[h.retryAttempts]))
}
h.retryAttempts++
@@ -302,22 +304,22 @@ func (s *MockArvadosServerSuite) TestWithRetries(c *C) {
"create", 0, 200, []int{200, 500}, []string{`{"ok":"ok"}`, ``},
},
{
- "get", 0, 500, []int{500, 500, 500, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
+ "get", 0, 423, []int{500, 500, 423, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
},
{
- "create", 0, 500, []int{500, 500, 500, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
+ "create", 0, 423, []int{500, 500, 423, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
},
{
- "update", 0, 500, []int{500, 500, 500, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
+ "update", 0, 422, []int{500, 500, 422, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
},
{
- "delete", 0, 500, []int{500, 500, 500, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
+ "delete", 0, 422, []int{500, 500, 422, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
},
{
- "get", 0, 502, []int{500, 500, 502, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
+ "get", 0, 401, []int{500, 502, 401, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
},
{
- "create", 0, 502, []int{500, 500, 502, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
+ "create", 0, 422, []int{500, 502, 422, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
},
{
"get", 0, 200, []int{500, 500, 200}, []string{``, ``, `{"ok":"ok"}`},
@@ -337,6 +339,12 @@ func (s *MockArvadosServerSuite) TestWithRetries(c *C) {
{
"create", 0, 401, []int{401, 200}, []string{``, `{"ok":"ok"}`},
},
+ {
+ "create", 0, 403, []int{403, 200}, []string{``, `{"ok":"ok"}`},
+ },
+ {
+ "create", 0, 422, []int{422, 200}, []string{``, `{"ok":"ok"}`},
+ },
{
"get", 0, 404, []int{404, 200}, []string{``, `{"ok":"ok"}`},
},
@@ -352,11 +360,13 @@ func (s *MockArvadosServerSuite) TestWithRetries(c *C) {
{
"get", 0, 200, []int{-1, -1, 200}, []string{``, ``, `{"ok":"ok"}`},
},
- // "POST" is not safe to retry: fail after one error
+ // "POST" protocol error is safe to retry
{
- "create", 0, -1, []int{-1, 200}, []string{``, `{"ok":"ok"}`},
+ "create", 0, 200, []int{-1, 200}, []string{``, `{"ok":"ok"}`},
},
} {
+ c.Logf("stub: %#v", stub)
+
api, err := RunFakeArvadosServer(&stub)
c.Check(err, IsNil)
@@ -396,7 +406,9 @@ func (s *MockArvadosServerSuite) TestWithRetries(c *C) {
default:
c.Check(err, NotNil)
c.Check(err, ErrorMatches, fmt.Sprintf("arvados API server error: %d.*", stub.expected))
- c.Check(err.(APIServerError).HttpStatusCode, Equals, stub.expected)
+ if c.Check(err, FitsTypeOf, APIServerError{}) {
+ c.Check(err.(APIServerError).HttpStatusCode, Equals, stub.expected)
+ }
}
}
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list