Skip to content

Commit

Permalink
Simplify passing host configuration direct to Index client (#280)
Browse files Browse the repository at this point in the history
## Problem

In production situations, it's important people be able to configure the
client in a way that does not hit the control plane to find out the host
url. This was possible, but now that I'm writing docs I realized this is
cumbersome because the positional index name arg was still being
required even though it wasn't being used in that scenario.

We want it to be easy.

## Solution

### Before

```python
from pinecone.grpc import PineconeGRPC

pc = PineconeGRPC(api_key="key")

# Targeting by name is easy
index = pc.Index('blah')

# Targeting by host url is awkward
index = pc.Index('required-but-unused-index-name', host='blah-24vnhz6.svc.apw5-4e34-81fa.pinecone.io')
```

### After

```python
from pinecone.grpc import PineconeGRPC

pc = PineconeGRPC(api_key="key")

# This still works
index = pc.Index('blah')

# This now works
index = pc.Index(host='blah-24vnhz6.svc.apw5-4e34-81fa.pinecone.io')

# Or pass both if you really want, but not needed.
index = pc.Index(name='blah', host='blah-24vnhz6.svc.apw5-4e34-81fa.pinecone.io')
```

## Type of Change

- [x] New feature (non-breaking change which adds functionality)

## Test Plan

Update integration tests to check it can still issue data calls without
errors no matter how it is configured.
  • Loading branch information
jhamon committed Jan 15, 2024
1 parent 557afd5 commit 44fc7ed
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 11 deletions.
18 changes: 13 additions & 5 deletions pinecone/control/pinecone.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from pinecone.core.client.api.manage_indexes_api import ManageIndexesApi as IndexOperationsApi
from pinecone.core.client.api_client import ApiClient
from pinecone.utils import get_user_agent
from pinecone.utils import get_user_agent, normalize_host
from pinecone.core.client.models import (
CreateCollectionRequest,
CreateIndexRequest,
Expand Down Expand Up @@ -213,7 +213,15 @@ def _get_status(self, name: str):
response = api_instance.describe_index(name)
return response["status"]

def Index(self, name: str, host: Optional[str] = None):
if host is None:
host = self.index_host_store.get_host(self.index_api, self.config, name)
return Index(api_key=self.config.api_key, host=host, pool_threads=self.pool_threads)
def Index(self, name: str = '', host: str = ''):
if name == '' and host == '':
raise ValueError("Either name or host must be specified")

if host != '':
# Use host url if it is provided
return Index(api_key=self.config.api_key, host=normalize_host(host), pool_threads=self.pool_threads)

if name != '':
# Otherwise, get host url from describe_index using the index name
index_host = self.index_host_store.get_host(self.index_api, self.config, name)
return Index(api_key=self.config.api_key, host=index_host, pool_threads=self.pool_threads)
20 changes: 14 additions & 6 deletions pinecone/grpc/pinecone.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
from ..control.pinecone import Pinecone
from ..config.config import ConfigBuilder
from .index_grpc import GRPCIndex
from typing import Optional

class PineconeGRPC(Pinecone):
def Index(self, name: str, host: Optional[str] = None):
if host is None:
host = self.index_host_store.get_host(self.index_api, self.config, name)
config = ConfigBuilder.build(api_key=self.config.api_key, host=host)
return GRPCIndex(index_name=name, config=config)
def Index(self, name: str = '', host: str = ''):
if name == '' and host == '':
raise ValueError("Either name or host must be specified")

if host != '':
# Use host if it is provided
config = ConfigBuilder.build(api_key=self.config.api_key, host=host)
return GRPCIndex(index_name=name, config=config)

if name != '':
# Otherwise, get host url from describe_index using the index name
index_host = self.index_host_store.get_host(self.index_api, self.config, name)
config = ConfigBuilder.build(api_key=self.config.api_key, host=index_host)
return GRPCIndex(index_name=name, config=config)
38 changes: 38 additions & 0 deletions tests/integration/data/test_initialization.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import pytest

class TestIndexClientInitialization:
def test_index_direct_host_kwarg(self, client, index_host):
index = client.Index(host=index_host)
index.fetch(ids=['1', '2', '3'])

def test_index_direct_host_with_https(self, client, index_host):
if not index_host.startswith('https://'):
index_host = 'https://' + index_host
index = client.Index(host=index_host)
index.fetch(ids=['1', '2', '3'])

def test_index_direct_host_without_https(self, client, index_host):
if index_host.startswith('https://'):
index_host = index_host[8:]
index = client.Index(host=index_host)
index.fetch(ids=['1', '2', '3'])

def test_index_by_name_positional_only(self, client, index_name, index_host):
index = client.Index(index_name)
index.fetch(ids=['1', '2', '3'])

def test_index_by_name_positional_with_host(self, client, index_name, index_host):
index = client.Index(index_name, index_host)
index.fetch(ids=['1', '2', '3'])

def test_index_by_name_kwargs(self, client, index_name):
index = client.Index(name=index_name)
index.fetch(ids=['1', '2', '3'])

def test_index_by_name_kwargs_with_host(self, client, index_name, index_host):
index = client.Index(name=index_name, host=index_host)
index.fetch(ids=['1', '2', '3'])

def test_raises_when_no_name_or_host(self, client, index_host):
with pytest.raises(ValueError):
client.Index()

0 comments on commit 44fc7ed

Please sign in to comment.