X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/06c9ea02435fb56e8c32a31694a414ca161dc2ce..bdc069a04fd98529f5c79c6b8a7164fb9119723d:/lib/controller/federation/list.go diff --git a/lib/controller/federation/list.go b/lib/controller/federation/list.go index 4bd32fac20..0a596eb9cb 100644 --- a/lib/controller/federation/list.go +++ b/lib/controller/federation/list.go @@ -12,8 +12,8 @@ import ( "sync" "sync/atomic" - "git.curoverse.com/arvados.git/sdk/go/arvados" - "git.curoverse.com/arvados.git/sdk/go/httpserver" + "git.arvados.org/arvados.git/sdk/go/arvados" + "git.arvados.org/arvados.git/sdk/go/httpserver" ) //go:generate go run generate.go @@ -21,7 +21,7 @@ import ( // CollectionList is used as a template to auto-generate List() // methods for other types; see generate.go. -func (conn *Conn) CollectionList(ctx context.Context, options arvados.ListOptions) (arvados.CollectionList, error) { +func (conn *Conn) generated_CollectionList(ctx context.Context, options arvados.ListOptions) (arvados.CollectionList, error) { var mtx sync.Mutex var merged arvados.CollectionList var needSort atomic.Value @@ -52,6 +52,12 @@ func (conn *Conn) CollectionList(ctx context.Context, options arvados.ListOption return mj.Before(mi) }) } + if merged.Items == nil { + // Return empty results as [], not null + // (https://github.com/golang/go/issues/27589 might be + // a better solution in the future) + merged.Items = []arvados.Collection{} + } return merged, err } @@ -78,7 +84,7 @@ func (conn *Conn) CollectionList(ctx context.Context, options arvados.ListOption // // * len(Order)==0 // -// * Each filter must be either "uuid = ..." or "uuid in [...]". +// * Each filter is either "uuid = ..." or "uuid in [...]". // // * The maximum possible response size (total number of objects that // could potentially be matched by all of the specified filters) @@ -100,6 +106,13 @@ func (conn *Conn) CollectionList(ctx context.Context, options arvados.ListOption // corresponding options argument suitable for sending to that // backend. func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions, fn func(context.Context, string, arvados.API, arvados.ListOptions) ([]string, error)) error { + + if opts.BypassFederation { + // Client requested no federation. Pass through. + _, err := fn(ctx, conn.cluster.ClusterID, conn.local, opts) + return err + } + cannotSplit := false var matchAllFilters map[string]bool for _, f := range opts.Filters { @@ -175,28 +188,27 @@ func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions } } - if len(todoByRemote) > 1 { - if cannotSplit { - return httpErrorf(http.StatusBadRequest, "cannot execute federated list query: each filter must be either 'uuid = ...' or 'uuid in [...]'") - } - if opts.Count != "none" { - return httpErrorf(http.StatusBadRequest, "cannot execute federated list query unless count==\"none\"") - } - if opts.Limit >= 0 || opts.Offset != 0 || len(opts.Order) > 0 { - return httpErrorf(http.StatusBadRequest, "cannot execute federated list query with limit, offset, or order parameter") - } - if max := conn.cluster.API.MaxItemsPerResponse; nUUIDs > max { - return httpErrorf(http.StatusBadRequest, "cannot execute federated list query because number of UUIDs (%d) exceeds page size limit %d", nUUIDs, max) - } - selectingUUID := false - for _, attr := range opts.Select { - if attr == "uuid" { - selectingUUID = true - } - } - if opts.Select != nil && !selectingUUID { - return httpErrorf(http.StatusBadRequest, "cannot execute federated list query with a select parameter that does not include uuid") - } + if len(todoByRemote) == 0 { + return nil + } + if len(todoByRemote) == 1 && todoByRemote[conn.cluster.ClusterID] != nil { + // All UUIDs are local, so proxy a single request. The + // generic case has some limitations (see below) which + // we don't want to impose on local requests. + _, err := fn(ctx, conn.cluster.ClusterID, conn.local, opts) + return err + } + if cannotSplit { + return httpErrorf(http.StatusBadRequest, "cannot execute federated list query: each filter must be either 'uuid = ...' or 'uuid in [...]'") + } + if opts.Count != "none" { + return httpErrorf(http.StatusBadRequest, "cannot execute federated list query unless count==\"none\"") + } + if opts.Limit >= 0 || opts.Offset != 0 || len(opts.Order) > 0 { + return httpErrorf(http.StatusBadRequest, "cannot execute federated list query with limit, offset, or order parameter") + } + if max := conn.cluster.API.MaxItemsPerResponse; nUUIDs > max { + return httpErrorf(http.StatusBadRequest, "cannot execute federated list query because number of UUIDs (%d) exceeds page size limit %d", nUUIDs, max) } ctx, cancel := context.WithCancel(ctx) @@ -219,6 +231,12 @@ func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions return } remoteOpts := opts + if remoteOpts.Select != nil { + // We always need to select UUIDs to + // use the response, even if our + // caller doesn't. + remoteOpts.Select = append([]string{"uuid"}, remoteOpts.Select...) + } for len(todo) > 0 { if len(batch) > len(todo) { // Reduce batch to just the todo's @@ -231,7 +249,7 @@ func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions done, err := fn(ctx, clusterID, backend, remoteOpts) if err != nil { - errs <- err + errs <- httpErrorf(http.StatusBadGateway, err.Error()) return } progress := false @@ -241,8 +259,13 @@ func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions delete(todo, uuid) } } - if !progress { - errs <- httpErrorf(http.StatusBadGateway, "cannot make progress in federated list query: cluster %q returned none of the requested UUIDs", clusterID) + if len(done) == 0 { + // Zero items == no more + // results exist, no need to + // get another page. + break + } else if !progress { + errs <- httpErrorf(http.StatusBadGateway, "cannot make progress in federated list query: cluster %q returned %d items but none had the requested UUIDs", clusterID, len(done)) return } }