Adjust config kwargs to Pinecone and PineconeAsyncio (#447)

## Problem

We want to deprecate a few kwargs and give more helpful error messages.

## Solution

- Deprecate `config` and `openapi_config` kwargs; as far as I know these
were only used for tests. They have not appeared in documentation. It
adds quite a bit of complexity trying to merge configuration from all
these sources so going forward we would prefer to just expect
configuration from a single source, which are named kwargs.
- Add `NotImplementedError` messages that clearly explain some features
that are not implemented yet for PineconeAsyncio. This is preferrable to
allowing users to pass kwargs that have no effect, then getting
frustrated when they seem not to work.
- Give more informative error when it looks like the user has passed an
invalid value as the host

## Type of Change

- [x] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [x] This change requires a documentation update

Author: jhamon

pinecone/control/pinecone.py

@@ -6,7 +6,7 @@
 from .index_host_store import IndexHostStore
 from .pinecone_interface import PineconeDBControlInterface
 
-from pinecone.config import PineconeConfig, Config, ConfigBuilder
+from pinecone.config import PineconeConfig, ConfigBuilder
 
 from pinecone.core.openapi.db_control.api.manage_indexes_api import ManageIndexesApi
 from pinecone.openapi_support.api_client import ApiClient
@@ -59,33 +59,30 @@ def __init__(
         proxy_headers: Optional[Dict[str, str]] = None,
         ssl_ca_certs: Optional[str] = None,
         ssl_verify: Optional[bool] = None,
-        config: Optional[Config] = None,
         additional_headers: Optional[Dict[str, str]] = {},
         pool_threads: Optional[int] = None,
         **kwargs,
     ):
-        if config:
-            if not isinstance(config, Config):
-                raise TypeError("config must be of type pinecone.config.Config")
-            else:
-                self.config = config
-        else:
-            self.config = PineconeConfig.build(
-                api_key=api_key,
-                host=host,
-                additional_headers=additional_headers,
-                proxy_url=proxy_url,
-                proxy_headers=proxy_headers,
-                ssl_ca_certs=ssl_ca_certs,
-                ssl_verify=ssl_verify,
-                **kwargs,
+        if kwargs.get("config", None):
+            raise Exception(
+                "Passing config is no longer supported. Please pass individual settings such as proxy_url, proxy_headers, ssl_ca_certs, and ssl_verify directly to the Pinecone constructor as keyword arguments. See the README at https://github.com/pinecone-io/pinecone-python-client for examples."
             )
-
         if kwargs.get("openapi_config", None):
             raise Exception(
-                "Passing openapi_config is no longer supported. Please pass settings such as proxy_url, proxy_headers, ssl_ca_certs, and ssl_verify directly to the Pinecone constructor as keyword arguments. See the README at https://github.com/pinecone-io/pinecone-python-client for examples."
+                "Passing openapi_config is no longer supported. Please pass individual settings such as proxy_url, proxy_headers, ssl_ca_certs, and ssl_verify directly to the Pinecone constructor as keyword arguments. See the README at https://github.com/pinecone-io/pinecone-python-client for examples."
             )
 
+        self.config = PineconeConfig.build(
+            api_key=api_key,
+            host=host,
+            additional_headers=additional_headers,
+            proxy_url=proxy_url,
+            proxy_headers=proxy_headers,
+            ssl_ca_certs=ssl_ca_certs,
+            ssl_verify=ssl_verify,
+            **kwargs,
+        )
+
         self.openapi_config = ConfigBuilder.build_openapi_config(self.config, **kwargs)
 
         if pool_threads is None:
@@ -300,6 +297,8 @@ def Index(self, name: str = "", host: str = "", **kwargs):
         openapi_config = self.openapi_config
 
         if host != "":
+            check_realistic_host(host)
+
             # Use host url if it is provided
             index_host = normalize_host(host)
         else:
@@ -314,3 +313,19 @@ def Index(self, name: str = "", host: str = "", **kwargs):
             source_tag=self.config.source_tag,
             **kwargs,
         )
+
+
+def check_realistic_host(host: str) -> None:
+    """@private
+
+    Checks whether a user-provided host string seems plausible.
+    Someone could erroneously pass an index name as the host by
+    mistake, and if they have done that we'd like to give them a
+    simple error message as feedback rather than attempting to
+    call the url and getting a more cryptic DNS resolution error.
+    """
+
+    if "." not in host and "localhost" not in host:
+        raise ValueError(
+            f"You passed '{host}' as the host but this does not appear to be valid. Call describe_index() to confirm the host of the index."
+        )

pinecone/control/pinecone_asyncio.py

@@ -33,6 +33,7 @@
 from .types import CreateIndexForModelEmbedTypedDict
 from .request_factory import PineconeDBControlRequestFactory
 from .pinecone_interface_asyncio import PineconeAsyncioDBControlInterface
+from .pinecone import check_realistic_host
 
 logger = logging.getLogger(__name__)
 """ @private """
@@ -50,21 +51,33 @@ def __init__(
         self,
         api_key: Optional[str] = None,
         host: Optional[str] = None,
-        proxy_url: Optional[str] = None,
-        proxy_headers: Optional[Dict[str, str]] = None,
-        ssl_ca_certs: Optional[str] = None,
-        ssl_verify: Optional[bool] = None,
-        additional_headers: Optional[Dict[str, str]] = {},
+        # proxy_url: Optional[str] = None,
+        # proxy_headers: Optional[Dict[str, str]] = None,
+        # ssl_ca_certs: Optional[str] = None,
+        # ssl_verify: Optional[bool] = None,
+        # additional_headers: Optional[Dict[str, str]] = {},
         **kwargs,
     ):
+        for kwarg in {
+            "additional_headers",
+            "proxy_url",
+            "proxy_headers",
+            "ssl_ca_certs",
+            "ssl_verify",
+        }:
+            if kwarg in kwargs:
+                raise NotImplementedError(
+                    f"You have passed {kwarg} but this configuration has not been implemented yet for PineconeAsyncio."
+                )
+
         self.config = PineconeConfig.build(
             api_key=api_key,
             host=host,
-            additional_headers=additional_headers,
-            proxy_url=proxy_url,
-            proxy_headers=proxy_headers,
-            ssl_ca_certs=ssl_ca_certs,
-            ssl_verify=ssl_verify,
+            additional_headers=None,
+            proxy_url=None,
+            proxy_headers=None,
+            ssl_ca_certs=None,
+            ssl_verify=None,
             **kwargs,
         )
         self.openapi_config = ConfigBuilder.build_openapi_config(self.config, **kwargs)
@@ -257,9 +270,11 @@ def Index(self, host: str, **kwargs):
         api_key = self.config.api_key
         openapi_config = self.openapi_config
 
+        if host is None or host == "":
+            raise ValueError("A host must be specified")
+
+        check_realistic_host(host)
         index_host = normalize_host(host)
-        if index_host == "":
-            raise ValueError("host must be specified")
 
         return _AsyncioIndex(
             host=index_host,

pinecone/control/pinecone_interface_asyncio.py

@@ -529,7 +529,7 @@ async def describe_collection(self, name: str):
         pass
 
     @abstractmethod
-    async def Index(self, host: str, **kwargs):
+    def Index(self, host: str, **kwargs):
         """
         Target an index for data operations.
 

tests/integration/control/serverless/test_index_instantiation_ux.py

@@ -5,7 +5,7 @@
 class TestIndexInstantiationUX:
     def test_index_instantiation_ux(self):
         with pytest.raises(Exception) as e:
-            pinecone.Index(name="my-index", host="host")
+            pinecone.Index(name="my-index", host="test-bt8x3su.svc.apw5-4e34-81fa.pinecone.io")
 
         assert (
             "You are attempting to access the Index client directly from the pinecone module."

tests/unit/data/test_instantiation.py

@@ -0,0 +1,26 @@
+import pytest
+
+
+class TestIndexInstantiation:
+    def test_invalid_host(self):
+        from pinecone import Pinecone
+
+        pc = Pinecone(api_key="key")
+
+        with pytest.raises(ValueError) as e:
+            pc.Index(host="invalid")
+        assert "You passed 'invalid' as the host but this does not appear to be valid" in str(
+            e.value
+        )
+
+        with pytest.raises(ValueError) as e:
+            pc.Index(host="my-index")
+        assert "You passed 'my-index' as the host but this does not appear to be valid" in str(
+            e.value
+        )
+
+        # Can instantiate with realistic host
+        pc.Index(host="test-bt8x3su.svc.apw5-4e34-81fa.pinecone.io")
+
+        # Can instantiate with localhost address
+        pc.Index(host="localhost:8080")

tests/unit/test_config.py

@@ -100,9 +100,11 @@ def test_errors_when_no_api_key_is_present(self):
             PineconeConfig.build()
 
     def test_config_pool_threads(self):
-        pc = Pinecone(api_key="test-api-key", host="test-controller-host", pool_threads=10)
+        pc = Pinecone(
+            api_key="test-api-key", host="test-controller-host.pinecone.io", pool_threads=10
+        )
         assert pc.index_api.api_client.pool_threads == 10
-        idx = pc.Index(host="my-index-host", name="my-index-name")
+        idx = pc.Index(host="my-index-host.pinecone.io", name="my-index-name")
         assert idx._vector_api.api_client.pool_threads == 10
 
     def test_ssl_config_passed_to_index_client(self):
@@ -112,7 +114,7 @@ def test_ssl_config_passed_to_index_client(self):
         assert pc.openapi_config.ssl_ca_cert == "path/to/cert"
         assert pc.openapi_config.proxy_headers == proxy_headers
 
-        idx = pc.Index(host="host")
+        idx = pc.Index(host="host.pinecone.io")
         assert idx._vector_api.api_client.configuration.ssl_ca_cert == "path/to/cert"
         assert idx._vector_api.api_client.configuration.proxy_headers == proxy_headers
 
@@ -124,10 +126,10 @@ def test_host_config_not_clobbered_by_index(self):
         assert pc.openapi_config.proxy_headers == proxy_headers
         assert pc.openapi_config.host == "https://api.pinecone.io"
 
-        idx = pc.Index(host="host")
+        idx = pc.Index(host="host.pinecone.io")
         assert idx._vector_api.api_client.configuration.ssl_ca_cert == "path/to/cert"
         assert idx._vector_api.api_client.configuration.proxy_headers == proxy_headers
-        assert idx._vector_api.api_client.configuration.host == "https://host"
+        assert idx._vector_api.api_client.configuration.host == "https://host.pinecone.io"
 
         assert pc.openapi_config.host == "https://api.pinecone.io"
 

tests/unit/test_config_builder.py

@@ -7,27 +7,27 @@
 
 class TestConfigBuilder:
     def test_build_simple(self):
-        config = ConfigBuilder.build(api_key="my-api-key", host="https://my-controller-host")
+        config = ConfigBuilder.build(api_key="my-api-key", host="https://my-controller-host.com")
         assert config.api_key == "my-api-key"
-        assert config.host == "https://my-controller-host"
+        assert config.host == "https://my-controller-host.com"
         assert config.additional_headers == {}
 
     def test_build_merges_key_and_host_when_openapi_config_provided(self):
         config = ConfigBuilder.build(
             api_key="my-api-key",
-            host="https://my-controller-host",
+            host="https://my-controller-host.com",
             openapi_config=OpenApiConfiguration(),
         )
         assert config.api_key == "my-api-key"
-        assert config.host == "https://my-controller-host"
+        assert config.host == "https://my-controller-host.com"
         assert config.additional_headers == {}
 
     def test_build_with_source_tag(self):
         config = ConfigBuilder.build(
-            api_key="my-api-key", host="https://my-controller-host", source_tag="my-source-tag"
+            api_key="my-api-key", host="https://my-controller-host.com", source_tag="my-source-tag"
         )
         assert config.api_key == "my-api-key"
-        assert config.host == "https://my-controller-host"
+        assert config.host == "https://my-controller-host.com"
         assert config.additional_headers == {}
         assert config.source_tag == "my-source-tag"
 
@@ -42,35 +42,35 @@ def test_build_errors_when_no_host_is_present(self):
         assert str(e.value) == "You haven't specified a host."
 
     def test_build_openapi_config(self):
-        config = ConfigBuilder.build(api_key="my-api-key", host="https://my-controller-host")
+        config = ConfigBuilder.build(api_key="my-api-key", host="https://my-controller-host.com")
         openapi_config = ConfigBuilder.build_openapi_config(config)
-        assert openapi_config.host == "https://my-controller-host"
+        assert openapi_config.host == "https://my-controller-host.com"
         assert openapi_config.api_key == {"ApiKeyAuth": "my-api-key"}
 
     def test_build_openapi_config_merges_with_existing_config(self):
-        config = ConfigBuilder.build(api_key="my-api-key", host="https://my-controller-host")
+        config = ConfigBuilder.build(api_key="my-api-key", host="https://my-controller-host.com")
         openapi_config = OpenApiConfiguration()
         openapi_config.ssl_ca_cert = "path/to/bundle"
         openapi_config.proxy = "http://my-proxy:8080"
 
         openapi_config = ConfigBuilder.build_openapi_config(config, openapi_config)
 
         assert openapi_config.api_key == {"ApiKeyAuth": "my-api-key"}
-        assert openapi_config.host == "https://my-controller-host"
+        assert openapi_config.host == "https://my-controller-host.com"
         assert openapi_config.ssl_ca_cert == "path/to/bundle"
         assert openapi_config.proxy == "http://my-proxy:8080"
 
     def test_build_openapi_config_does_not_mutate_input(self):
         config = ConfigBuilder.build(
-            api_key="my-api-key", host="foo", ssl_ca_certs="path/to/bundle.foo"
+            api_key="my-api-key", host="foo.pinecone.io", ssl_ca_certs="path/to/bundle.foo"
         )
 
         input_openapi_config = OpenApiConfiguration()
         input_openapi_config.host = "bar"
         input_openapi_config.ssl_ca_cert = "asdfasdf"
 
         openapi_config = ConfigBuilder.build_openapi_config(config, input_openapi_config)
-        assert openapi_config.host == "https://foo"
+        assert openapi_config.host == "https://foo.pinecone.io"
         assert openapi_config.ssl_ca_cert == "path/to/bundle.foo"
 
         assert input_openapi_config.host == "bar"

tests/unit/test_control.py

@@ -2,7 +2,6 @@
 import re
 from unittest.mock import patch, MagicMock
 from pinecone import (
-    ConfigBuilder,
     Pinecone,
     PodSpec,
     ServerlessSpec,
@@ -34,7 +33,7 @@ def description_with_status(status: bool):
         status=IndexModelStatus(ready=status, state=state),
         dimension=10,
         deletion_protection=DeletionProtection(value="enabled"),
-        host="https://foo",
+        host="https://foo.pinecone.io",
         metric="euclidean",
         spec=IndexModelSpec(serverless=ServerlessSpecOpenApi(cloud="aws", region="us-west1")),
     )
@@ -48,7 +47,7 @@ def index_list_response():
                 name="index1",
                 dimension=10,
                 metric="euclidean",
-                host="asdf",
+                host="asdf.pinecone.io",
                 status={"ready": True},
                 spec={},
                 deletion_protection=DeletionProtection("enabled"),
@@ -58,7 +57,7 @@ def index_list_response():
                 name="index2",
                 dimension=10,
                 metric="euclidean",
-                host="asdf",
+                host="asdf.pinecone.io",
                 status={"ready": True},
                 spec={},
                 deletion_protection=DeletionProtection("enabled"),
@@ -68,7 +67,7 @@ def index_list_response():
                 name="index3",
                 dimension=10,
                 metric="euclidean",
-                host="asdf",
+                host="asdf.pinecone.io",
                 status={"ready": True},
                 spec={},
                 deletion_protection=DeletionProtection("disabled"),
@@ -89,8 +88,8 @@ def test_default_host(self):
         assert p.index_api.api_client.configuration.host == "https://api.pinecone.io"
 
     def test_passing_host(self):
-        p = Pinecone(api_key="123-456-789", host="my-host")
-        assert p.index_api.api_client.configuration.host == "https://my-host"
+        p = Pinecone(api_key="123-456-789", host="my-host.pinecone.io")
+        assert p.index_api.api_client.configuration.host == "https://my-host.pinecone.io"
 
     def test_passing_additional_headers(self):
         extras = {"header1": "my-value", "header2": "my-value2"}
@@ -119,13 +118,6 @@ def test_set_source_tag_in_useragent(self):
             re.search(r"source_tag=test_source_tag", p.index_api.api_client.user_agent) is not None
         )
 
-    def test_set_source_tag_in_useragent_via_config(self):
-        config = ConfigBuilder.build(
-            api_key="YOUR_API_KEY", host="https://my-host", source_tag="my_source_tag"
-        )
-        p = Pinecone(config=config)
-        assert re.search(r"source_tag=my_source_tag", p.index_api.api_client.user_agent) is not None
-
     @pytest.mark.parametrize(
         "timeout_value, describe_index_responses, expected_describe_index_calls, expected_sleep_calls",
         [