[ARVADOS] updated: 624c5c8d13d6c2e21a80379928387944bedae2a3
git at public.curoverse.com
git at public.curoverse.com
Thu Apr 30 10:24:09 EDT 2015
Summary of changes:
sdk/go/arvadosclient/arvadosclient.go | 64 +++++++++++++++++++++---------
sdk/go/arvadosclient/arvadosclient_test.go | 14 ++++---
services/keepstore/handlers.go | 47 +++++++++++++---------
3 files changed, 81 insertions(+), 44 deletions(-)
via 624c5c8d13d6c2e21a80379928387944bedae2a3 (commit)
via a4211acb465bd42869bf2a2f9fad6ff2c5e518e0 (commit)
via 8af4c5ffe2d0bf7f009f8821945aa100ddf02279 (commit)
via e24940267012211db40ca0adcedfc33650314b70 (commit)
via 0191b58223f99476cc8553f3ad7c726a4d8b37de (commit)
via de5f63b3abc80783ebe886078c5e4bcfa19ed1da (commit)
from 6c9dde5e6089efc8a87022ab9771da96932516a7 (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 624c5c8d13d6c2e21a80379928387944bedae2a3
Merge: a4211ac 8af4c5f
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Apr 30 10:25:13 2015 -0400
Merge branch '3145-close-early' closes #3145
commit a4211acb465bd42869bf2a2f9fad6ff2c5e518e0
Merge: 6c9dde5 e249402
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Apr 30 10:24:16 2015 -0400
Merge branch '5836-remote-api-server-errors-made-obvious' closes #5836
commit e24940267012211db40ca0adcedfc33650314b70
Author: mishaz <misha at curoverse.com>
Date: Wed Apr 29 16:31:00 2015 +0000
Renamed RemoteApiServerError to APIServerError and NewRemoteApiServerError to newAPIServerError.
Reworked error messages to feature error details more prominently.
These changes were requested by code review at
https://github.com/curoverse/arvados/pull/17/files
diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go
index 1cd565e..ae40a89 100644
--- a/sdk/go/arvadosclient/arvadosclient.go
+++ b/sdk/go/arvadosclient/arvadosclient.go
@@ -21,7 +21,7 @@ var MissingArvadosApiHost = errors.New("Missing required environment variable AR
var MissingArvadosApiToken = errors.New("Missing required environment variable ARVADOS_API_TOKEN")
// Indicates an error that was returned by the API server.
-type RemoteApiServerError struct {
+type APIServerError struct {
// Address of server returning error, of the form "host:port".
ServerAddress string
@@ -33,17 +33,19 @@ type RemoteApiServerError struct {
ErrorDetails []string
}
-func (e RemoteApiServerError) Error() string {
- s := fmt.Sprintf("API server (%s) returned %d: %s.",
- e.ServerAddress,
- e.HttpStatusCode,
- e.HttpStatusMessage)
+func (e APIServerError) Error() string {
if len(e.ErrorDetails) > 0 {
- s = fmt.Sprintf("%s Additional details: %s",
- s,
- strings.Join(e.ErrorDetails, "; "))
+ return fmt.Sprintf("arvados API server error: %s (%d: %s) returned by %s",
+ strings.Join(e.ErrorDetails, "; "),
+ e.HttpStatusCode,
+ e.HttpStatusMessage,
+ e.ServerAddress)
+ } else {
+ return fmt.Sprintf("arvados API server error: %d: %s returned by %s",
+ e.HttpStatusCode,
+ e.HttpStatusMessage,
+ e.ServerAddress)
}
- return s
}
// Helper type so we don't have to write out 'map[string]interface{}' every time.
@@ -167,12 +169,12 @@ func (this ArvadosClient) CallRaw(method string, resource string, uuid string, a
}
defer resp.Body.Close()
- return nil, NewRemoteApiServerError(this.ApiServer, resp)
+ return nil, newAPIServerError(this.ApiServer, resp)
}
-func NewRemoteApiServerError(ServerAddress string, resp *http.Response) RemoteApiServerError {
+func newAPIServerError(ServerAddress string, resp *http.Response) APIServerError {
- rase := RemoteApiServerError{
+ ase := APIServerError{
ServerAddress: ServerAddress,
HttpStatusCode: resp.StatusCode,
HttpStatusMessage: resp.Status}
@@ -188,15 +190,15 @@ func NewRemoteApiServerError(ServerAddress string, resp *http.Response) RemoteAp
// Non-strings will be passed along
// JSON-encoded.
if s, ok := errItem.(string); ok {
- rase.ErrorDetails = append(rase.ErrorDetails, s)
+ ase.ErrorDetails = append(ase.ErrorDetails, s)
} else if j, err := json.Marshal(errItem); err == nil {
- rase.ErrorDetails = append(rase.ErrorDetails, string(j))
+ ase.ErrorDetails = append(ase.ErrorDetails, string(j))
}
}
}
}
}
- return rase
+ return ase
}
// Access to a resource.
diff --git a/sdk/go/arvadosclient/arvadosclient_test.go b/sdk/go/arvadosclient/arvadosclient_test.go
index 4810675..21eff20 100644
--- a/sdk/go/arvadosclient/arvadosclient_test.go
+++ b/sdk/go/arvadosclient/arvadosclient_test.go
@@ -86,19 +86,19 @@ func (s *ServerRequiredSuite) TestErrorResponse(c *C) {
err := arv.Create("logs",
Dict{"log": Dict{"bogus_attr": "foo"}},
&getback)
- c.Assert(err, ErrorMatches, "API Server.*")
+ c.Assert(err, ErrorMatches, "arvados API server error: .*")
c.Assert(err, ErrorMatches, ".*unknown attribute: bogus_attr.*")
- c.Assert(err, FitsTypeOf, RemoteApiServerError{})
- c.Assert(err.(RemoteApiServerError).HttpStatusCode, Equals, 422)
+ c.Assert(err, FitsTypeOf, APIServerError{})
+ c.Assert(err.(APIServerError).HttpStatusCode, Equals, 422)
}
{
err := arv.Create("bogus",
Dict{"bogus": Dict{}},
&getback)
- c.Assert(err, ErrorMatches, "API Server.*")
+ c.Assert(err, ErrorMatches, "arvados API server error: .*")
c.Assert(err, ErrorMatches, ".*Path not found.*")
- c.Assert(err, FitsTypeOf, RemoteApiServerError{})
- c.Assert(err.(RemoteApiServerError).HttpStatusCode, Equals, 404)
+ c.Assert(err, FitsTypeOf, APIServerError{})
+ c.Assert(err.(APIServerError).HttpStatusCode, Equals, 404)
}
}
commit 0191b58223f99476cc8553f3ad7c726a4d8b37de
Author: mishaz <misha at curoverse.com>
Date: Tue Apr 28 22:29:27 2015 +0000
Added tests to confirm that error messages indicate that error is remote.
Fixed punctuation.
diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go
index 2f5ea5e..1cd565e 100644
--- a/sdk/go/arvadosclient/arvadosclient.go
+++ b/sdk/go/arvadosclient/arvadosclient.go
@@ -29,7 +29,7 @@ type RemoteApiServerError struct {
HttpStatusCode int
HttpStatusMessage string
- // Additional error details from response body
+ // Additional error details from response body.
ErrorDetails []string
}
diff --git a/sdk/go/arvadosclient/arvadosclient_test.go b/sdk/go/arvadosclient/arvadosclient_test.go
index f7d0cc9..4810675 100644
--- a/sdk/go/arvadosclient/arvadosclient_test.go
+++ b/sdk/go/arvadosclient/arvadosclient_test.go
@@ -86,6 +86,7 @@ func (s *ServerRequiredSuite) TestErrorResponse(c *C) {
err := arv.Create("logs",
Dict{"log": Dict{"bogus_attr": "foo"}},
&getback)
+ c.Assert(err, ErrorMatches, "API Server.*")
c.Assert(err, ErrorMatches, ".*unknown attribute: bogus_attr.*")
c.Assert(err, FitsTypeOf, RemoteApiServerError{})
c.Assert(err.(RemoteApiServerError).HttpStatusCode, Equals, 422)
@@ -95,6 +96,7 @@ func (s *ServerRequiredSuite) TestErrorResponse(c *C) {
err := arv.Create("bogus",
Dict{"bogus": Dict{}},
&getback)
+ c.Assert(err, ErrorMatches, "API Server.*")
c.Assert(err, ErrorMatches, ".*Path not found.*")
c.Assert(err, FitsTypeOf, RemoteApiServerError{})
c.Assert(err.(RemoteApiServerError).HttpStatusCode, Equals, 404)
commit de5f63b3abc80783ebe886078c5e4bcfa19ed1da
Author: mishaz <misha at curoverse.com>
Date: Tue Apr 28 22:16:46 2015 +0000
Rewrote ArvadosApiError as RemoteApiServerError:
* Name change to indicate that it's a remote error, not a local one.
* Rewrote Error() method to explicitly state that the error is remote and list all fields.
* Added server address field and use it in error response.
* Error details are now kept as a string slice rather than as a delimited string.
Some cleanup and gofmt import shuffling.
diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go
index 4c16398..2f5ea5e 100644
--- a/sdk/go/arvadosclient/arvadosclient.go
+++ b/sdk/go/arvadosclient/arvadosclient.go
@@ -20,13 +20,31 @@ import (
var MissingArvadosApiHost = errors.New("Missing required environment variable ARVADOS_API_HOST")
var MissingArvadosApiToken = errors.New("Missing required environment variable ARVADOS_API_TOKEN")
-type ArvadosApiError struct {
- error
- HttpStatusCode int
- HttpStatus string
+// Indicates an error that was returned by the API server.
+type RemoteApiServerError struct {
+ // Address of server returning error, of the form "host:port".
+ ServerAddress string
+
+ // Components of server response.
+ HttpStatusCode int
+ HttpStatusMessage string
+
+ // Additional error details from response body
+ ErrorDetails []string
}
-func (e ArvadosApiError) Error() string { return e.error.Error() }
+func (e RemoteApiServerError) Error() string {
+ s := fmt.Sprintf("API server (%s) returned %d: %s.",
+ e.ServerAddress,
+ e.HttpStatusCode,
+ e.HttpStatusMessage)
+ if len(e.ErrorDetails) > 0 {
+ s = fmt.Sprintf("%s Additional details: %s",
+ s,
+ strings.Join(e.ErrorDetails, "; "))
+ }
+ return s
+}
// Helper type so we don't have to write out 'map[string]interface{}' every time.
type Dict map[string]interface{}
@@ -50,15 +68,15 @@ type ArvadosClient struct {
External bool
}
-// Create a new KeepClient, initialized with standard Arvados environment
+// Create a new ArvadosClient, initialized with standard Arvados environment
// variables ARVADOS_API_HOST, ARVADOS_API_TOKEN, and (optionally)
// ARVADOS_API_HOST_INSECURE.
-func MakeArvadosClient() (kc ArvadosClient, err error) {
+func MakeArvadosClient() (ac ArvadosClient, err error) {
var matchTrue = regexp.MustCompile("^(?i:1|yes|true)$")
insecure := matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE"))
external := matchTrue.MatchString(os.Getenv("ARVADOS_EXTERNAL_CLIENT"))
- kc = ArvadosClient{
+ ac = ArvadosClient{
ApiServer: os.Getenv("ARVADOS_API_HOST"),
ApiToken: os.Getenv("ARVADOS_API_TOKEN"),
ApiInsecure: insecure,
@@ -66,14 +84,14 @@ func MakeArvadosClient() (kc ArvadosClient, err error) {
TLSClientConfig: &tls.Config{InsecureSkipVerify: insecure}}},
External: external}
- if kc.ApiServer == "" {
- return kc, MissingArvadosApiHost
+ if ac.ApiServer == "" {
+ return ac, MissingArvadosApiHost
}
- if kc.ApiToken == "" {
- return kc, MissingArvadosApiToken
+ if ac.ApiToken == "" {
+ return ac, MissingArvadosApiToken
}
- return kc, err
+ return ac, err
}
// Low-level access to a resource.
@@ -149,30 +167,36 @@ func (this ArvadosClient) CallRaw(method string, resource string, uuid string, a
}
defer resp.Body.Close()
- errorText := fmt.Sprintf("API response: %s", resp.Status)
+ return nil, NewRemoteApiServerError(this.ApiServer, resp)
+}
+
+func NewRemoteApiServerError(ServerAddress string, resp *http.Response) RemoteApiServerError {
+
+ rase := RemoteApiServerError{
+ ServerAddress: ServerAddress,
+ HttpStatusCode: resp.StatusCode,
+ HttpStatusMessage: resp.Status}
// If the response body has {"errors":["reason1","reason2"]}
// then return those reasons.
var errInfo = Dict{}
if err := json.NewDecoder(resp.Body).Decode(&errInfo); err == nil {
if errorList, ok := errInfo["errors"]; ok {
- var errorStrings []string
if errArray, ok := errorList.([]interface{}); ok {
for _, errItem := range errArray {
// We expect an array of strings here.
// Non-strings will be passed along
// JSON-encoded.
if s, ok := errItem.(string); ok {
- errorStrings = append(errorStrings, s)
+ rase.ErrorDetails = append(rase.ErrorDetails, s)
} else if j, err := json.Marshal(errItem); err == nil {
- errorStrings = append(errorStrings, string(j))
+ rase.ErrorDetails = append(rase.ErrorDetails, string(j))
}
}
- errorText = strings.Join(errorStrings, "; ")
}
}
}
- return nil, ArvadosApiError{errors.New(errorText), resp.StatusCode, resp.Status}
+ return rase
}
// Access to a resource.
diff --git a/sdk/go/arvadosclient/arvadosclient_test.go b/sdk/go/arvadosclient/arvadosclient_test.go
index 1af964d..f7d0cc9 100644
--- a/sdk/go/arvadosclient/arvadosclient_test.go
+++ b/sdk/go/arvadosclient/arvadosclient_test.go
@@ -1,8 +1,8 @@
package arvadosclient
import (
- . "gopkg.in/check.v1"
"git.curoverse.com/arvados.git/sdk/go/arvadostest"
+ . "gopkg.in/check.v1"
"net/http"
"os"
"testing"
@@ -87,16 +87,16 @@ func (s *ServerRequiredSuite) TestErrorResponse(c *C) {
Dict{"log": Dict{"bogus_attr": "foo"}},
&getback)
c.Assert(err, ErrorMatches, ".*unknown attribute: bogus_attr.*")
- c.Assert(err, FitsTypeOf, ArvadosApiError{})
- c.Assert(err.(ArvadosApiError).HttpStatusCode, Equals, 422)
+ c.Assert(err, FitsTypeOf, RemoteApiServerError{})
+ c.Assert(err.(RemoteApiServerError).HttpStatusCode, Equals, 422)
}
{
err := arv.Create("bogus",
Dict{"bogus": Dict{}},
&getback)
- c.Assert(err, ErrorMatches, "Path not found")
- c.Assert(err, FitsTypeOf, ArvadosApiError{})
- c.Assert(err.(ArvadosApiError).HttpStatusCode, Equals, 404)
+ c.Assert(err, ErrorMatches, ".*Path not found.*")
+ c.Assert(err, FitsTypeOf, RemoteApiServerError{})
+ c.Assert(err.(RemoteApiServerError).HttpStatusCode, Equals, 404)
}
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list