Fix normalizeHost
, allow for non-secure connections
#74
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
When working with specific environments and hosts that require a non-secure connection, calling
pc.Index(pinecone.NewIndexConnParams{Host: idx.Host})
with aHost
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 agrpc.DialOption
.Our
normalizeHost
function has a bug in that it applies port:443
toHost
values which are passed without an explicit scheme.Solution
normalizeHost
function to only deal with stripping away any provided scheme, and then checking to see if the connection is explicitly insecure. This is accomplished by providinghttp://
as a part of theNewIndexConnParams{ Host: yourHost }
value, which would need to be accounted for in documentation, etc.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 explicitbool
as a part of theNewIndexConnParams
, 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 ingo.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
After