[ARVADOS] updated: 1.3.0-2377-g12b534152
Git user
git at public.arvados.org
Thu Mar 26 14:47:13 UTC 2020
Summary of changes:
lib/controller/federation/list_test.go | 8 ++++----
lib/controller/federation/user_test.go | 5 +++++
lib/controller/integration_test.go | 21 +++++++++++++++++++++
lib/controller/rpc/conn.go | 20 +++++++++++++-------
sdk/go/arvados/api.go | 4 ++--
5 files changed, 45 insertions(+), 13 deletions(-)
via 12b5341528770adc532b6c3e169036addd945d52 (commit)
via c56e9fb917c59ff1e6a601f58701d47ec336256b (commit)
via bca49fe61c6a819598005ea6afda1b3c69048ef3 (commit)
from 5c941aa10b7b4bf9d0804f98eb93a8104bfb7658 (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 12b5341528770adc532b6c3e169036addd945d52
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Thu Mar 26 11:46:34 2020 -0300
16263: Decodes JSON numbers as strings instead of float64.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go
index 4b143b770..b5c56dbc4 100644
--- a/lib/controller/rpc/conn.go
+++ b/lib/controller/rpc/conn.go
@@ -5,6 +5,7 @@
package rpc
import (
+ "bytes"
"context"
"crypto/tls"
"encoding/json"
@@ -14,6 +15,7 @@ import (
"net"
"net/http"
"net/url"
+ "strconv"
"strings"
"time"
@@ -100,19 +102,23 @@ func (conn *Conn) requestAndDecode(ctx context.Context, dst interface{}, ep arva
return fmt.Errorf("%T: requestAndDecode: Marshal opts: %s", conn, err)
}
var params map[string]interface{}
- err = json.Unmarshal(j, ¶ms)
+ dec := json.NewDecoder(bytes.NewBuffer(j))
+ dec.UseNumber()
+ err = dec.Decode(¶ms)
if err != nil {
- return fmt.Errorf("%T: requestAndDecode: Unmarshal opts: %s", conn, err)
+ return fmt.Errorf("%T: requestAndDecode: Decode opts: %s", conn, err)
}
if attrs, ok := params["attrs"]; ok && ep.AttrsKey != "" {
params[ep.AttrsKey] = attrs
delete(params, "attrs")
}
- if limit, ok := params["limit"].(float64); ok && limit < 0 {
- // Negative limit means "not specified" here, but some
- // servers/versions do not accept that, so we need to
- // remove it entirely.
- delete(params, "limit")
+ if limitStr, ok := params["limit"]; ok {
+ if limit, err := strconv.ParseInt(string(limitStr.(json.Number)), 10, 64); err == nil && limit < 0 {
+ // Negative limit means "not specified" here, but some
+ // servers/versions do not accept that, so we need to
+ // remove it entirely.
+ delete(params, "limit")
+ }
}
if len(tokens) > 1 {
params["reader_tokens"] = tokens[1:]
commit c56e9fb917c59ff1e6a601f58701d47ec336256b
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Wed Mar 25 16:35:18 2020 -0300
16263: Adds unit test case confirming 'limit' bug.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/lib/controller/federation/list_test.go b/lib/controller/federation/list_test.go
index ce84378a3..e6d2816f6 100644
--- a/lib/controller/federation/list_test.go
+++ b/lib/controller/federation/list_test.go
@@ -58,7 +58,7 @@ func (cl *collectionLister) CollectionList(ctx context.Context, options arvados.
if cl.MaxPageSize > 0 && len(resp.Items) >= cl.MaxPageSize {
break
}
- if options.Limit >= 0 && len(resp.Items) >= options.Limit {
+ if options.Limit >= 0 && int64(len(resp.Items)) >= options.Limit {
break
}
if cl.matchFilters(c, options.Filters) {
@@ -115,8 +115,8 @@ func (s *CollectionListSuite) SetUpTest(c *check.C) {
type listTrial struct {
count string
- limit int
- offset int
+ limit int64
+ offset int64
order []string
filters []arvados.Filter
selectfields []string
@@ -314,7 +314,7 @@ func (s *CollectionListSuite) TestCollectionListMultiSiteWithCount(c *check.C) {
}
func (s *CollectionListSuite) TestCollectionListMultiSiteWithLimit(c *check.C) {
- for _, limit := range []int{0, 1, 2} {
+ for _, limit := range []int64{0, 1, 2} {
s.test(c, listTrial{
count: "none",
limit: limit,
diff --git a/lib/controller/federation/user_test.go b/lib/controller/federation/user_test.go
index c087273af..c55ec24d4 100644
--- a/lib/controller/federation/user_test.go
+++ b/lib/controller/federation/user_test.go
@@ -7,6 +7,7 @@ package federation
import (
"encoding/json"
"errors"
+ "math"
"net/url"
"os"
"strings"
@@ -32,6 +33,7 @@ func (s *UserSuite) TestLoginClusterUserList(c *check.C) {
for _, updateFail := range []bool{false, true} {
for _, opts := range []arvados.ListOptions{
{Offset: 0, Limit: -1, Select: nil},
+ {Offset: 0, Limit: math.MaxInt64, Select: nil},
{Offset: 1, Limit: 1, Select: nil},
{Offset: 0, Limit: 2, Select: []string{"uuid"}},
{Offset: 0, Limit: 2, Select: []string{"uuid", "email"}},
@@ -45,6 +47,9 @@ func (s *UserSuite) TestLoginClusterUserList(c *check.C) {
s.fed.local = rpc.NewConn(s.cluster.ClusterID, spy.URL, true, rpc.PassthroughTokenProvider)
}
userlist, err := s.fed.UserList(s.ctx, opts)
+ if err != nil {
+ c.Logf("... UserList failed %q", err)
+ }
if updateFail && err == nil {
// All local updates fail, so the only
// cases expected to succeed are the
commit bca49fe61c6a819598005ea6afda1b3c69048ef3
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date: Wed Mar 25 11:26:55 2020 -0300
16263: Adds test exposing a bug when using 'limit' with the max int64 value.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>
diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index 19dd4e1ad..d2ae1f6fb 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -8,6 +8,7 @@ import (
"bytes"
"context"
"io"
+ "math"
"net"
"net/url"
"os"
@@ -252,3 +253,23 @@ func (s *IntegrationSuite) TestListUsers(c *check.C) {
_, err = conn3.UserList(rootctx1, arvados.ListOptions{Limit: -1})
c.Assert(err, check.IsNil, check.Commentf("getting user list: %q", err))
}
+
+// Test for bug #16263
+func (s *IntegrationSuite) TestListUsersWithMaxLimit(c *check.C) {
+ rootctx1, _, _ := s.rootClients("z1111")
+ conn3 := s.conn("z3333")
+ maxLimit := int64(math.MaxInt64)
+
+ // Make sure LoginCluster is properly configured
+ for cls := range s.testClusters {
+ c.Check(
+ s.testClusters[cls].config.Clusters[cls].Login.LoginCluster,
+ check.Equals, "z1111",
+ check.Commentf("incorrect LoginCluster config on cluster %q", cls))
+ }
+
+ // Ask for the user list on z3333 using z1111's system root token and
+ // limit: max int64 value.
+ _, err := conn3.UserList(rootctx1, arvados.ListOptions{Limit: maxLimit})
+ c.Assert(err, check.IsNil, check.Commentf("getting user list: %q", err))
+}
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index 0c5d32e8b..4eb5b61b3 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -77,8 +77,8 @@ type ListOptions struct {
Select []string `json:"select"`
Filters []Filter `json:"filters"`
Where map[string]interface{} `json:"where"`
- Limit int `json:"limit"`
- Offset int `json:"offset"`
+ Limit int64 `json:"limit"`
+ Offset int64 `json:"offset"`
Order []string `json:"order"`
Distinct bool `json:"distinct"`
Count string `json:"count"`
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list