Skip to content

do not override port if defined in host #362

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

Merged
merged 1 commit into from
Jul 3, 2024
Merged

do not override port if defined in host #362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions pinecone/grpc/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,10 @@ def stub_class(self):
pass

def _endpoint(self):
grpcHost = self.config.host.replace("https://", "")
return self._endpoint_override if self._endpoint_override else f"{grpcHost}:443"
grpc_host = self.config.host.replace("https://", "")
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this needs to be expanded to handle "http" as well. 🤔

Not that you need to fix that here, I just realized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It probably needs to be overhauled in general. I'll have another refactor to make it more holistic once it's needed for REST. This only fixes gRPC data plane config for now.

if ":" not in grpc_host:
grpc_host = f"{grpc_host}:443"
return self._endpoint_override if self._endpoint_override else grpc_host

def _gen_channel(self, options=None):
target = self._endpoint()
Expand Down
18 changes: 18 additions & 0 deletions tests/unit_grpc/test_grpc_index_initialization.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,24 @@ def test_config_passed_when_target_by_host(self):
assert index.grpc_client_config.reuse_channel == True
assert index.grpc_client_config.conn_timeout == 1

# Endpoint port defaults to 443
assert index._endpoint() == "myhost:443"

def test_config_passed_when_target_by_host_and_port(self):
pc = PineconeGRPC(api_key="YOUR_API_KEY")
config = GRPCClientConfig(timeout=5, secure=False)
index = pc.Index(host="myhost:4343", grpc_config=config)

assert index.grpc_client_config.timeout == 5
assert index.grpc_client_config.secure == False

# Unset fields still get default values
assert index.grpc_client_config.reuse_channel == True
assert index.grpc_client_config.conn_timeout == 1

# Endpoint calculation does not override port
assert index._endpoint() == "myhost:4343"

def test_config_passes_source_tag_when_set(self):
pc = PineconeGRPC(api_key="YOUR_API_KEY", source_tag="my_source_tag")
index = pc.Index(name="my-index", host="host")
Expand Down