Skip to content

Commit

Permalink
Address Austin comments
Browse files Browse the repository at this point in the history
  • Loading branch information
aulorbe committed Aug 2, 2024
1 parent e44b546 commit bccdc08
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 37 deletions.
10 changes: 5 additions & 5 deletions pinecone/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ func (c *Client) Index(in NewIndexConnParams, dialOpts ...grpc.DialOption) (*Ind
}

if in.Host == "" {
return nil, fmt.Errorf("host is required to create an IndexConnection. Find your host from calling DescribeIndex or via the Pinecone console")
return nil, fmt.Errorf("field Host is required to create an IndexConnection. Find your Host from calling DescribeIndex or via the Pinecone console")
}

// add api version header if not provided
Expand Down Expand Up @@ -509,7 +509,7 @@ func (req CreatePodIndexRequest) TotalCount() int {
// }
func (c *Client) CreatePodIndex(ctx context.Context, in *CreatePodIndexRequest) (*Index, error) {
if in.Name == "" || in.Dimension == 0 || in.Metric == "" || in.Environment == "" || in.PodType == "" {
return nil, fmt.Errorf("name, dimension, metric, environment, and podtype must be included in CreatePodIndexRequest")
return nil, fmt.Errorf("fields Name, Dimension, Metric, Environment, and Podtype must be included in CreatePodIndexRequest")
}

deletionProtection := pointerOrNil(control.DeletionProtection(in.DeletionProtection))
Expand Down Expand Up @@ -655,7 +655,7 @@ type CreateServerlessIndexRequest struct {
// }
func (c *Client) CreateServerlessIndex(ctx context.Context, in *CreateServerlessIndexRequest) (*Index, error) {
if in.Name == "" || in.Dimension == 0 || in.Metric == "" || in.Cloud == "" || in.Region == "" {
return nil, fmt.Errorf("name, dimension, metric, cloud, and region must be included in CreateServerlessIndexRequest")
return nil, fmt.Errorf("fields Name, Dimension, Metric, Cloud, and Region must be included in CreateServerlessIndexRequest")
}

deletionProtection := pointerOrNil(control.DeletionProtection(in.DeletionProtection))
Expand Down Expand Up @@ -1112,7 +1112,7 @@ type CreateCollectionRequest struct {
// [Collection]: https://docs.pinecone.io/guides/indexes/understanding-collections
func (c *Client) CreateCollection(ctx context.Context, in *CreateCollectionRequest) (*Collection, error) {
if in.Source == "" || in.Name == "" {
return nil, fmt.Errorf("name and source must be included in CreateCollectionRequest")
return nil, fmt.Errorf("fields Name and Source must be included in CreateCollectionRequest")
}

req := control.CreateCollectionRequest{
Expand Down Expand Up @@ -1343,7 +1343,7 @@ func formatError(errMap errorResponseMap) error {
}

func buildClientBaseOptions(in NewClientBaseParams) []control.ClientOption {
var clientOptions []control.ClientOption
clientOptions := []control.ClientOption{}

// build and apply user agent header
userAgentProvider := provider.NewHeaderProvider("User-Agent", useragent.BuildUserAgent(in.SourceTag))
Expand Down
45 changes: 21 additions & 24 deletions pinecone/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,14 +255,6 @@ func (ts *IntegrationTests) TestCreatePodIndex() {
require.Equal(ts.T(), name, idx.Name, "Index name does not match")
}

func (ts *IntegrationTests) TestCreatePodIndexMissingReqdFields() {
if ts.indexType == "serverless" {
ts.T().Skip("No pod index to test")
}
_, err := ts.client.CreatePodIndex(context.Background(), &CreatePodIndexRequest{})
require.ErrorContainsf(ts.T(), err, "name, dimension, metric, environment, and podtype must be included in CreatePodIndexRequest", err.Error())
}

func (ts *IntegrationTests) TestCreatePodIndexInvalidDimension() {
name := uuid.New().String()

Expand Down Expand Up @@ -310,14 +302,6 @@ func (ts *IntegrationTests) TestCreateServerlessIndex() {
require.Equal(ts.T(), name, idx.Name, "Index name does not match")
}

func (ts *IntegrationTests) TestCreateServerlessIndexMissingReqdFields() {
if ts.indexType == "pods" {
ts.T().Skip("No serverless index to test")
}
_, err := ts.client.CreateServerlessIndex(context.Background(), &CreateServerlessIndexRequest{})
require.ErrorContainsf(ts.T(), err, "name, dimension, metric, cloud, and region must be included in CreateServerlessIndexRequest", err.Error())
}

func (ts *IntegrationTests) TestDescribeServerlessIndex() {
if ts.indexType == "pods" {
ts.T().Skip("No serverless index to test")
Expand Down Expand Up @@ -449,14 +433,6 @@ func (ts *IntegrationTests) TestCreateCollection() {
require.Equal(ts.T(), name, collection.Name, "Collection name does not match")
}

func (ts *IntegrationTests) TestCreateCollectionMissingReqdFields() {
if ts.indexType == "serverless" {
ts.T().Skip("No pod index to test")
}
_, err := ts.client.CreateCollection(context.Background(), &CreateCollectionRequest{})
require.ErrorContains(ts.T(), err, "name and source must be included in CreateCollectionRequest")
}

func (ts *IntegrationTests) TestDeleteCollection() {
if ts.indexType == "serverless" {
ts.T().Skip("No pod index to test")
Expand Down Expand Up @@ -732,6 +708,27 @@ func (ts *IntegrationTests) TestNoHostPassedToIndexConnection() {
}

// Unit tests:
func TestCreatePodIndexMissingReqdFieldsUnit(t *testing.T) {
client := &Client{}
_, err := client.CreatePodIndex(context.Background(), &CreatePodIndexRequest{})
require.Error(t, err)
require.ErrorContainsf(t, err, "fields Name, Dimension, Metric, Environment, and Podtype must be included in CreatePodIndexRequest", err.Error()) //_, err := ts.client.CreatePodIndex(context.Background(), &CreatePodIndexRequest{})
}

func TestCreateServerlessIndexMissingReqdFieldsUnit(t *testing.T) {
client := &Client{}
_, err := client.CreateServerlessIndex(context.Background(), &CreateServerlessIndexRequest{})
require.Error(t, err)
require.ErrorContainsf(t, err, "fields Name, Dimension, Metric, Cloud, and Region must be included in CreateServerlessIndexRequest", err.Error())
}

func TestCreateCollectionMissingReqdFieldsUnit(t *testing.T) {
client := &Client{}
_, err := client.CreateCollection(context.Background(), &CreateCollectionRequest{})
require.Error(t, err)
require.ErrorContains(t, err, "fields Name and Source must be included in CreateCollectionRequest")
}

func TestHandleErrorResponseBodyUnit(t *testing.T) {
tests := []struct {
name string
Expand Down
2 changes: 1 addition & 1 deletion pinecone/index_connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -1088,7 +1088,7 @@ func sparseValToGrpc(sv *SparseValues) *data.SparseValues {
}

func (idx *IndexConnection) akCtx(ctx context.Context) context.Context {
var newMetadata []string
newMetadata := []string{}

for key, value := range idx.additionalMetadata {
newMetadata = append(newMetadata, key, value)
Expand Down
15 changes: 8 additions & 7 deletions pinecone/index_connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,13 +256,6 @@ func (ts *IntegrationTests) TestUpdateVectorValues() {
assert.ElementsMatch(ts.T(), expectedVals, actualVals, "Values do not match")
}

func (ts *IntegrationTests) TestUpdateVectorMissingReqdFields() {
ctx := context.Background()
err := ts.idxConn.UpdateVector(ctx, &UpdateVectorRequest{})
assert.Error(ts.T(), err)
assert.Contains(ts.T(), err.Error(), "a vector ID plus at least one of Values, SparseValues, or Metadata must be provided to update a vector")
}

func (ts *IntegrationTests) TestUpdateVectorMetadata() {
ctx := context.Background()

Expand Down Expand Up @@ -324,6 +317,14 @@ func (ts *IntegrationTests) TestUpdateVectorSparseValues() error {
}

// Unit tests:
func TestUpdateVectorMissingReqdFieldsUnit(t *testing.T) {
ctx := context.Background()
idxConn := &IndexConnection{}
err := idxConn.UpdateVector(ctx, &UpdateVectorRequest{})
assert.Error(t, err)
assert.Contains(t, err.Error(), "a vector ID plus at least one of Values, SparseValues, or Metadata must be provided to update a vector")
}

func TestMarshalFetchVectorsResponseUnit(t *testing.T) {
tests := []struct {
name string
Expand Down

0 comments on commit bccdc08

Please sign in to comment.