Skip to content

Commit

Permalink
more review feedback: rename URL scheme checker, remove extra API key…
Browse files Browse the repository at this point in the history
… env value, remove unnecessary test
  • Loading branch information
austin-denoble committed May 16, 2024
1 parent 99e6972 commit 7dea994
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 32 deletions.
1 change: 0 additions & 1 deletion .env.example
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
PINECONE_API_KEY="<Project API Key>"
TEST_PINECONE_API_KEY="<Test Project API Key>"
TEST_POD_INDEX_NAME="<Pod based Index name>"
TEST_SERVERLESS_INDEX_NAME="<Serverless based Index name>"
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ jobs:
env:
TEST_POD_INDEX_NAME: ${{ secrets.TEST_POD_INDEX_NAME }}
TEST_SERVERLESS_INDEX_NAME: ${{ secrets.TEST_SERVERLESS_INDEX_NAME }}
TEST_PINECONE_API_KEY: ${{ secrets.API_KEY }}
PINECONE_API_KEY: ${{ secrets.API_KEY }}
10 changes: 5 additions & 5 deletions pinecone/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type Client struct {
}

type NewClientParams struct {
ApiKey string // required
ApiKey string // required - provide through NewClientParams or environment variable PINECONE_API_KEY
Headers map[string]string // optional
Host string // optional
RestClient *http.Client // optional
Expand All @@ -39,10 +39,10 @@ type NewClientBaseParams struct {

func NewClient(in NewClientParams) (*Client, error) {
osApiKey := os.Getenv("PINECONE_API_KEY")
hasApiKey := (in.ApiKey != "" || osApiKey != "")
hasApiKey := (valueOrFallback(in.ApiKey, osApiKey) != "")

if !hasApiKey {
return nil, fmt.Errorf("no API key provided, please pass an API key for authorization")
return nil, fmt.Errorf("no API key provided, please pass an API key for authorization through NewClientParams or set the PINECONE_API_KEY environment variable")
}

apiKeyHeader := struct{ Key, Value string }{"Api-Key", valueOrFallback(in.ApiKey, osApiKey)}
Expand All @@ -65,7 +65,7 @@ func NewClientBase(in NewClientBaseParams) (*Client, error) {

controlHostOverride := valueOrFallback(in.Host, os.Getenv("PINECONE_CONTROLLER_HOST"))
if controlHostOverride != "" {
controlHostOverride, err = ensureHTTPS(controlHostOverride)
controlHostOverride, err = ensureURLScheme(controlHostOverride)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -512,7 +512,7 @@ func decodeCollection(resBody io.ReadCloser) (*Collection, error) {
return toCollection(&collectionModel), nil
}

func ensureHTTPS(inputURL string) (string, error) {
func ensureURLScheme(inputURL string) (string, error) {
parsedURL, err := url.Parse(inputURL)
if err != nil {
return "", fmt.Errorf("invalid URL: %v", err)
Expand Down
31 changes: 8 additions & 23 deletions pinecone/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@ func TestClient(t *testing.T) {
}

func (ts *ClientTests) SetupSuite() {
apiKey := os.Getenv("TEST_PINECONE_API_KEY")
require.NotEmpty(ts.T(), apiKey, "TEST_PINECONE_API_KEY env variable not set")
apiKey := os.Getenv("PINECONE_API_KEY")
require.NotEmpty(ts.T(), apiKey, "PINECONE_API_KEY env variable not set")

ts.podIndex = os.Getenv("TEST_POD_INDEX_NAME")
require.NotEmpty(ts.T(), ts.podIndex, "TEST_POD_INDEX_NAME env variable not set")

ts.serverlessIndex = os.Getenv("TEST_SERVERLESS_INDEX_NAME")
require.NotEmpty(ts.T(), ts.serverlessIndex, "TEST_SERVERLESS_INDEX_NAME env variable not set")

client, err := NewClient(NewClientParams{ApiKey: apiKey})
client, err := NewClient(NewClientParams{})
require.NoError(ts.T(), err)

ts.client = *client
Expand Down Expand Up @@ -97,13 +97,18 @@ func (ts *ClientTests) TestNewClientParamsSetHeaders() {
}

func (ts *ClientTests) TestNewClientParamsNoApiKeyNoAuthorizationHeader() {
apiKey := os.Getenv("PINECONE_API_KEY")
os.Unsetenv("PINECONE_API_KEY")

client, err := NewClient(NewClientParams{})
require.NotNil(ts.T(), err, "Expected error when creating client without an API key or Authorization header")
if !strings.Contains(err.Error(), "no API key provided, please pass an API key for authorization") {
ts.FailNow(fmt.Sprintf("Expected error to contain 'no API key provided, please pass an API key for authorization', but got '%s'", err.Error()))
}

require.Nil(ts.T(), client, "Expected client to be nil when creating client without an API key or Authorization header")

os.Setenv("PINECONE_API_KEY", apiKey)
}

func (ts *ClientTests) TestHeadersAppliedToRequests() {
Expand Down Expand Up @@ -170,26 +175,6 @@ func (ts *ClientTests) TestHeadersOverrideAdditionalHeaders() {
os.Unsetenv("PINECONE_ADDITIONAL_HEADERS")
}

func (ts *ClientTests) TestClientReadsApiKeyFromEnv() {
os.Setenv("PINECONE_API_KEY", "test-env-api-key")

httpClient := mocks.CreateMockClient(`{"indexes": []}`)
client, err := NewClient(NewClientParams{RestClient: httpClient})
if err != nil {
ts.FailNow(err.Error())
}
mockTransport := httpClient.Transport.(*mocks.MockTransport)

_, err = client.ListIndexes(context.Background())
require.NoError(ts.T(), err)
require.NotNil(ts.T(), mockTransport.Req, "Expected request to be made")

testHeaderValue := mockTransport.Req.Header.Get("Api-Key")
assert.Equal(ts.T(), "test-env-api-key", testHeaderValue, "Expected request to have header value 'test-env-api-key', but got '%s'", testHeaderValue)

os.Unsetenv("PINECONE_API_KEY")
}

func (ts *ClientTests) TestControllerHostOverride() {
apiKey := "test-api-key"
httpClient := mocks.CreateMockClient(`{"indexes": []}`)
Expand Down
4 changes: 2 additions & 2 deletions pinecone/index_connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ type IndexConnectionTests struct {

// Runs the test suite with `go test`
func TestIndexConnection(t *testing.T) {
apiKey := os.Getenv("TEST_PINECONE_API_KEY")
assert.NotEmptyf(t, apiKey, "API_KEY env variable not set")
apiKey := os.Getenv("PINECONE_API_KEY")
assert.NotEmptyf(t, apiKey, "PINECONE_API_KEY env variable not set")

client, err := NewClient(NewClientParams{ApiKey: apiKey})
if err != nil {
Expand Down

0 comments on commit 7dea994

Please sign in to comment.