Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add json:"" marshaling annotations to response structs #21

Merged
merged 3 commits into from
May 7, 2024
Merged

Add json:"" marshaling annotations to response structs #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c8204c
add json marshaling annotations to our response structs to facillitat…
austin-denoble May 3, 2024
567551d
tweak index_connection_test tests
austin-denoble May 3, 2024
4db920c
review feedback, fix newly failing delete by filter test, tweak addit…
austin-denoble May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions pinecone/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,12 +386,13 @@ func toCollection(cm *control.CollectionModel) *Collection {
if cm == nil {
return nil
}

return &Collection{
Name: cm.Name,
Size: cm.Size,
Size: derefOrDefault(cm.Size, 0),
Status: CollectionStatus(cm.Status),
Dimension: cm.Dimension,
VectorCount: cm.VectorCount,
Dimension: derefOrDefault(cm.Dimension, 0),
VectorCount: derefOrDefault(cm.VectorCount, 0),
Environment: cm.Environment,
}
}
Expand All @@ -413,6 +414,13 @@ func minOne(x int32) int32 {
return x
}

func derefOrDefault[T any](ptr *T, defaultValue T) T {
if ptr == nil {
return defaultValue
}
return *ptr
}

func buildClientOptions(in NewClientParams) ([]control.ClientOption, error) {
clientOptions := []control.ClientOption{}
hasAuthorizationHeader := false
Expand Down
45 changes: 23 additions & 22 deletions pinecone/index_connection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@ import (
)

type IndexConnection struct {
Namespace string
apiKey string
Namespace string
apiKey string
additionalMetadata map[string]string
dataClient *data.VectorServiceClient
grpcConn *grpc.ClientConn
dataClient *data.VectorServiceClient
grpcConn *grpc.ClientConn
}

type newIndexParameters struct {
apiKey string
host string
namespace string
sourceTag string
apiKey string
host string
namespace string
sourceTag string
additionalMetadata map[string]string
}

Expand Down Expand Up @@ -75,8 +75,8 @@ func (idx *IndexConnection) UpsertVectors(ctx context.Context, in []*Vector) (ui
}

type FetchVectorsResponse struct {
Vectors map[string]*Vector
Usage *Usage
Vectors map[string]*Vector `json:"vectors,omitempty"`
Usage *Usage `json:"usage,omitempty"`
}

func (idx *IndexConnection) FetchVectors(ctx context.Context, ids []string) (*FetchVectorsResponse, error) {
Expand All @@ -94,6 +94,7 @@ func (idx *IndexConnection) FetchVectors(ctx context.Context, ids []string) (*Fe
for id, vector := range res.Vectors {
vectors[id] = toVector(vector)
}
fmt.Printf("VECTORS: %+v\n", vectors)

return &FetchVectorsResponse{
Vectors: vectors,
Expand All @@ -108,9 +109,9 @@ type ListVectorsRequest struct {
}

type ListVectorsResponse struct {
VectorIds []*string
Usage *Usage
NextPaginationToken *string
VectorIds []*string `json:"vector_ids,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: your question in the PR description, from what I've read at least it seems like adding omitempty is a good call. I think it avoids ambiguous values -- like is a vector ID 0, or does it not exist? I'm always a fan of being explicit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are places where not using omitempty does make sense as it'll drop zero values that maybe you want to include.

For this PR I was mainly focused on making sure we're using omitempty for pointers and structs where it makes sense, primarily to avoid null values in the resulting JSON.

Usage *Usage `json:"usage,omitempty"`
NextPaginationToken *string `json:"next_pagination_token,omitempty"`
}

func (idx *IndexConnection) ListVectors(ctx context.Context, in *ListVectorsRequest) (*ListVectorsResponse, error) {
Expand All @@ -132,7 +133,7 @@ func (idx *IndexConnection) ListVectors(ctx context.Context, in *ListVectorsRequ

return &ListVectorsResponse{
VectorIds: vectorIds,
Usage: &Usage{ReadUnits: res.Usage.ReadUnits},
Usage: &Usage{ReadUnits: derefOrDefault(res.Usage.ReadUnits, 0)},
NextPaginationToken: toPaginationToken(res.Pagination),
}, nil
}
Expand All @@ -147,8 +148,8 @@ type QueryByVectorValuesRequest struct {
}

type QueryVectorsResponse struct {
Matches []*ScoredVector
Usage *Usage
Matches []*ScoredVector `json:"matches,omitempty"`
Usage *Usage `json:"usage,omitempty"`
}

func (idx *IndexConnection) QueryByVectorValues(ctx context.Context, in *QueryByVectorValuesRequest) (*QueryVectorsResponse, error) {
Expand Down Expand Up @@ -236,10 +237,10 @@ func (idx *IndexConnection) UpdateVector(ctx context.Context, in *UpdateVectorRe
}

type DescribeIndexStatsResponse struct {
Dimension uint32
IndexFullness float32
TotalVectorCount uint32
Namespaces map[string]*NamespaceSummary
Dimension uint32 `json:"dimension"`
IndexFullness float32 `json:"index_fullness"`
TotalVectorCount uint32 `json:"total_vector_count"`
Namespaces map[string]*NamespaceSummary `json:"namespaces,omitempty"`
}

func (idx *IndexConnection) DescribeIndexStats(ctx context.Context) (*DescribeIndexStatsResponse, error) {
Expand Down Expand Up @@ -335,7 +336,7 @@ func toUsage(u *data.Usage) *Usage {
return nil
}
return &Usage{
ReadUnits: u.ReadUnits,
ReadUnits: derefOrDefault(u.ReadUnits, 0),
}
}

Expand Down Expand Up @@ -372,7 +373,7 @@ func (idx *IndexConnection) akCtx(ctx context.Context) context.Context {
newMetadata := []string{}
newMetadata = append(newMetadata, "api-key", idx.apiKey)

for key, value := range idx.additionalMetadata{
for key, value := range idx.additionalMetadata {
newMetadata = append(newMetadata, key, value)
}

Expand Down
Loading
Loading