pinecone-io / go-pinecone

Pinecone.io Golang Client
Apache License 2.0
49 stars 9 forks source link

Fix `normalizeHost`, allow for non-secure connections #74

Closed austin-denoble closed 2 months ago

austin-denoble commented 2 months ago

Problem

When working with specific environments and hosts that require a non-secure connection, calling pc.Index(pinecone.NewIndexConnParams{Host: idx.Host}) with a Host value that is insecure, you'll run into unexpected behavior seeing :443 attached as a port, or gRPC errors around connecting insecurely without explicitly passing a grpc.DialOption.

Our normalizeHost function has a bug in that it applies port :443 to Host values which are passed without an explicit scheme.

Solution

This felt most straightforward because of how the grpc module handles host addresses. Ultimately, we need to strip the scheme off any hosts that are passed, but we also need to decide whether we're dialing in a secure or insecure fashion. I thought of providing an explicit bool as a part of the NewIndexConnParams, but it felt more unwieldy adding new fields.

Type of Change

Test Plan

I have a local test harness that I'm using to interact with a locally hosted index. I was overriding the go-pinecone package in go.mod to work against my local copy of the SDK. You can pull this branch down and do something similar locally. Here are some of my results:

Before

Screenshot 2024-09-12 at 10 09 45 PM

After

Screenshot 2024-09-12 at 10 33 59 PM