Skip to content
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.

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

Implement deletion protection #367

Merged
merged 14 commits into from
Jul 18, 2024
6 changes: 3 additions & 3 deletions .github/workflows/testing-integration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,17 @@ jobs:
uses: ./.github/actions/setup-poetry
- name: 'Run integration tests (REST, prod)'
if: matrix.pineconeEnv == 'prod'
run: poetry run pytest tests/integration/control/serverless -s -v
run: poetry run pytest tests/integration/control/pod -s -v
env:
PINECONE_DEBUG_CURL: 'true'
PINECONE_API_KEY: '${{ secrets.PINECONE_API_KEY }}'
PINECONE_ENVIRONMENT: '${{ matrix.testConfig.pod.environment }}'
GITHUB_BUILD_NUMBER: '${{ github.run_number }}-s-${{ matrix.testConfig.python-version}}'
DIMENSION: 1536
DIMENSION: 2500
METRIC: 'cosine'
- name: 'Run integration tests (REST, staging)'
if: matrix.pineconeEnv == 'staging'
run: poetry run pytest tests/integration/control/serverless -s -v
run: poetry run pytest tests/integration/control/pod -s -v
env:
PINECONE_DEBUG_CURL: 'true'
PINECONE_CONTROLLER_HOST: 'https://api-staging.pinecone.io'
Expand Down
2 changes: 1 addition & 1 deletion codegen/apis
Submodule apis updated from fbd9d8 to e9b47c
4 changes: 4 additions & 0 deletions codegen/build-oas.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ update_apis_repo() {
echo "Updating apis repo"
pushd codegen/apis
git fetch
git checkout main
git pull
just build
popd
Expand All @@ -21,6 +22,7 @@ update_templates_repo() {
echo "Updating templates repo"
pushd codegen/python-oas-templates
git fetch
git checkout main
git pull
popd
}
Expand Down Expand Up @@ -126,6 +128,8 @@ extract_shared_classes() {
done
done
done

echo "API_VERSION = '${version}'" > "${target_directory}/__init__.py"
}

update_apis_repo
Expand Down
4 changes: 0 additions & 4 deletions pinecone/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,4 @@
from .data import *
from .models import *

from .core.openapi.control.models import (
IndexModel,
)

from .utils import __version__
56 changes: 41 additions & 15 deletions pinecone/control/pinecone.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import time
import warnings
import logging
from typing import Optional, Dict, Any, Union, List, Tuple, cast, NamedTuple
from typing import Optional, Dict, Any, Union, List, Tuple, Literal

from .index_host_store import IndexHostStore

Expand All @@ -23,12 +23,14 @@
ConfigureIndexRequest,
ConfigureIndexRequestSpec,
ConfigureIndexRequestSpecPod,
DeletionProtection,
IndexSpec,
ServerlessSpec as ServerlessSpecModel,
PodSpec as PodSpecModel,
PodSpecMetadataConfig,
)
from pinecone.models import ServerlessSpec, PodSpec, IndexList, CollectionList
from pinecone.core.openapi.shared import API_VERSION
from pinecone.models import ServerlessSpec, PodSpec, IndexModel, IndexList, CollectionList
from .langchain_import_warnings import _build_langchain_attribute_error_message

from pinecone.data import Index
Expand Down Expand Up @@ -229,6 +231,7 @@ def __init__(
config=self.config,
openapi_config=self.openapi_config,
pool_threads=pool_threads,
api_version=API_VERSION,
)

self.index_host_store = IndexHostStore()
Expand Down Expand Up @@ -258,6 +261,7 @@ def create_index(
spec: Union[Dict, ServerlessSpec, PodSpec],
metric: Optional[str] = "cosine",
timeout: Optional[int] = None,
deletion_protection: Optional[Literal["enabled", "disabled"]] = "disabled",
):
"""Creates a Pinecone index.

Expand All @@ -278,6 +282,7 @@ def create_index(
:type timeout: int, optional
:param timeout: Specify the number of seconds to wait until index gets ready. If None, wait indefinitely; if >=0, time out after this many seconds;
if -1, return immediately and do not wait. Default: None
:param deletion_protection: If enabled, the index cannot be deleted. If disabled, the index can be deleted. Default: "disabled"

### Creating a serverless index

Expand All @@ -291,7 +296,8 @@ def create_index(
name="my_index",
dimension=1536,
metric="cosine",
spec=ServerlessSpec(cloud="aws", region="us-west-2")
spec=ServerlessSpec(cloud="aws", region="us-west-2"),
deletion_protection="enabled"
Comment on lines +299 to +300
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is docstrings code.

)
```

Expand All @@ -310,7 +316,8 @@ def create_index(
spec=PodSpec(
environment="us-east1-gcp",
pod_type="p1.x1"
)
),
deletion_protection="enabled"
)
```
"""
Expand All @@ -320,6 +327,11 @@ def create_index(
def _parse_non_empty_args(args: List[Tuple[str, Any]]) -> Dict[str, Any]:
return {arg_name: val for arg_name, val in args if val is not None}

if deletion_protection in ["enabled", "disabled"]:
dp = DeletionProtection(deletion_protection)
else:
raise ValueError("deletion_protection must be either 'enabled' or 'disabled'")
Comment on lines +330 to +333
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The generated DeletionProtection class implements a validation, but the error message it emits is very confusing. So I implemented my own check on this here and in the configure method.


if isinstance(spec, dict):
if "serverless" in spec:
index_spec = IndexSpec(serverless=ServerlessSpecModel(**spec["serverless"]))
Expand Down Expand Up @@ -376,6 +388,7 @@ def _parse_non_empty_args(args: List[Tuple[str, Any]]) -> Dict[str, Any]:
dimension=dimension,
metric=metric,
spec=index_spec,
deletion_protection=dp,
),
)

Expand Down Expand Up @@ -454,7 +467,7 @@ def list_indexes(self) -> IndexList:
index name, dimension, metric, status, and spec.

:return: Returns an `IndexList` object, which is iterable and contains a
list of `IndexDescription` objects. It also has a convenience method `names()`
list of `IndexModel` objects. It also has a convenience method `names()`
which returns a list of index names.

```python
Expand Down Expand Up @@ -498,7 +511,7 @@ def describe_index(self, name: str):
"""Describes a Pinecone index.

:param name: the name of the index to describe.
:return: Returns an `IndexDescription` object
:return: Returns an `IndexModel` object
which gives access to properties such as the
index name, dimension, metric, host url, status,
and spec.
Expand Down Expand Up @@ -530,13 +543,14 @@ def describe_index(self, name: str):
host = description.host
self.index_host_store.set_host(self.config, name, host)

return description
return IndexModel(description)

def configure_index(
self,
name: str,
replicas: Optional[int] = None,
pod_type: Optional[str] = None,
deletion_protection: Optional[Literal["enabled", "disabled"]] = None,
):
"""This method is used to scale configuration fields for your pod-based Pinecone index.

Expand All @@ -561,15 +575,28 @@ def configure_index(

"""
api_instance = self.index_api
config_args: Dict[str, Any] = {}

if deletion_protection is None:
description = self.describe_index(name=name)
dp = DeletionProtection(description.deletion_protection)
Comment on lines +579 to +581
Copy link
Collaborator Author

@jhamon jhamon Jul 18, 2024

Choose a reason for hiding this comment

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

If the user does not specify a value for deletion_protection, we call describe_index to look up the current value and pass that value with the other arguments in the configure request. Even though the API doesn't require a value to be passed, manual testing showed the generated code will stupidly plugin the default value of disabled if none is specified. This is needed to avoid disabling deletion protection when somebody is trying to scale by calling this with replicas or pod_type only.

elif deletion_protection in ["enabled", "disabled"]:
dp = DeletionProtection(deletion_protection)
else:
raise ValueError("deletion_protection must be either 'enabled' or 'disabled'")

pod_config_args: Dict[str, Any] = {}
if pod_type:
config_args.update(pod_type=pod_type)
pod_config_args.update(pod_type=pod_type)
if replicas:
config_args.update(replicas=replicas)
configure_index_request = ConfigureIndexRequest(
spec=ConfigureIndexRequestSpec(pod=ConfigureIndexRequestSpecPod(**config_args))
)
api_instance.configure_index(name, configure_index_request=configure_index_request)
pod_config_args.update(replicas=replicas)

if pod_config_args != {}:
spec = ConfigureIndexRequestSpec(pod=ConfigureIndexRequestSpecPod(**pod_config_args))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I hate the arrangement of this code. Blame the black formatter. I need to update the configs to shorten the line length to something more reasonable.

req = ConfigureIndexRequest(deletion_protection=dp, spec=spec)
else:
req = ConfigureIndexRequest(deletion_protection=dp)

api_instance.configure_index(name, configure_index_request=req)

def create_collection(self, name: str, source: str):
"""Create a collection from a pod-based index
Expand Down Expand Up @@ -634,7 +661,6 @@ def describe_collection(self, name: str):
print(description.source)
print(description.status)
print(description.size)
print(description.)
```
"""
api_instance = self.index_api
Expand Down
2 changes: 1 addition & 1 deletion pinecone/core/openapi/control/api/inference_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
)
from pinecone.core.openapi.control.model.embed_request import EmbedRequest
from pinecone.core.openapi.control.model.embeddings_list import EmbeddingsList
from pinecone.core.openapi.control.model.inline_response401 import InlineResponse401
from pinecone.core.openapi.control.model.error_response import ErrorResponse


class InferenceApi(object):
Expand Down
6 changes: 3 additions & 3 deletions pinecone/core/openapi/control/api/manage_indexes_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
from pinecone.core.openapi.control.model.configure_index_request import ConfigureIndexRequest
from pinecone.core.openapi.control.model.create_collection_request import CreateCollectionRequest
from pinecone.core.openapi.control.model.create_index_request import CreateIndexRequest
from pinecone.core.openapi.control.model.error_response import ErrorResponse
from pinecone.core.openapi.control.model.index_list import IndexList
from pinecone.core.openapi.control.model.index_model import IndexModel
from pinecone.core.openapi.control.model.inline_response401 import InlineResponse401


class ManageIndexesApi(object):
Expand All @@ -46,7 +46,7 @@ def __init__(self, api_client=None):
def __configure_index(self, index_name, configure_index_request, **kwargs):
"""Configure an index # noqa: E501

This operation specifies the pod type and number of replicas for an index. It applies to pod-based indexes only. Serverless indexes scale automatically based on usage. # noqa: E501
This operation configures an existing index. For serverless indexes, you can configure only index deletion protection. For pod-based indexes, you can configure the pod size, number of replicas, and index deletion protection. It is not possible to change the pod type of a pod-based index. However, you can create a collection from a pod-based index and then [create a new pod-based index with a different pod type](http://docs.pinecone.io/guides/indexes/create-an-index#create-an-index-from-a-collection) from the collection. For guidance and examples, see [Configure an index](http://docs.pinecone.io/guides/indexes/configure-an-index). # noqa: E501
This method makes a synchronous HTTP request by default. To make an
asynchronous HTTP request, please pass async_req=True

Expand All @@ -55,7 +55,7 @@ def __configure_index(self, index_name, configure_index_request, **kwargs):

Args:
index_name (str): The name of the index to configure.
configure_index_request (ConfigureIndexRequest): The desired pod type and replica configuration for the index.
configure_index_request (ConfigureIndexRequest): The desired pod size and replica configuration for the index.

Keyword Args:
_return_http_data_only (bool): response data without head status
Expand Down
2 changes: 1 addition & 1 deletion pinecone/core/openapi/control/model/collection_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class CollectionModel(ModelNormal):

validations = {
("dimension",): {
"inclusive_maximum": 2000,
"inclusive_maximum": 20000,
"inclusive_minimum": 1,
},
}
Expand Down
20 changes: 10 additions & 10 deletions pinecone/core/openapi/control/model/configure_index_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@

def lazy_import():
from pinecone.core.openapi.control.model.configure_index_request_spec import ConfigureIndexRequestSpec
from pinecone.core.openapi.control.model.deletion_protection import DeletionProtection

globals()["ConfigureIndexRequestSpec"] = ConfigureIndexRequestSpec
globals()["DeletionProtection"] = DeletionProtection


class ConfigureIndexRequest(ModelNormal):
Expand Down Expand Up @@ -97,6 +99,7 @@ def openapi_types():
lazy_import()
return {
"spec": (ConfigureIndexRequestSpec,), # noqa: E501
"deletion_protection": (DeletionProtection,), # noqa: E501
}

@cached_property
Expand All @@ -105,6 +108,7 @@ def discriminator():

attribute_map = {
"spec": "spec", # noqa: E501
"deletion_protection": "deletion_protection", # noqa: E501
}

read_only_vars = {}
Expand All @@ -113,12 +117,9 @@ def discriminator():

@classmethod
@convert_js_args_to_python_args
def _from_openapi_data(cls, spec, *args, **kwargs): # noqa: E501
def _from_openapi_data(cls, *args, **kwargs): # noqa: E501
"""ConfigureIndexRequest - a model defined in OpenAPI

Args:
spec (ConfigureIndexRequestSpec):

Keyword Args:
_check_type (bool): if True, values for parameters in openapi_types
will be type checked and a TypeError will be
Expand Down Expand Up @@ -150,6 +151,8 @@ def _from_openapi_data(cls, spec, *args, **kwargs): # noqa: E501
Animal class but this time we won't travel
through its discriminator because we passed in
_visited_composed_classes = (Animal,)
spec (ConfigureIndexRequestSpec): [optional] # noqa: E501
deletion_protection (DeletionProtection): [optional] # noqa: E501
"""

_check_type = kwargs.pop("_check_type", True)
Expand Down Expand Up @@ -178,7 +181,6 @@ def _from_openapi_data(cls, spec, *args, **kwargs): # noqa: E501
self._configuration = _configuration
self._visited_composed_classes = _visited_composed_classes + (self.__class__,)

self.spec = spec
for var_name, var_value in kwargs.items():
if (
var_name not in self.attribute_map
Expand All @@ -203,12 +205,9 @@ def _from_openapi_data(cls, spec, *args, **kwargs): # noqa: E501
)

@convert_js_args_to_python_args
def __init__(self, spec, *args, **kwargs): # noqa: E501
def __init__(self, *args, **kwargs): # noqa: E501
"""ConfigureIndexRequest - a model defined in OpenAPI

Args:
spec (ConfigureIndexRequestSpec):

Keyword Args:
_check_type (bool): if True, values for parameters in openapi_types
will be type checked and a TypeError will be
Expand Down Expand Up @@ -240,6 +239,8 @@ def __init__(self, spec, *args, **kwargs): # noqa: E501
Animal class but this time we won't travel
through its discriminator because we passed in
_visited_composed_classes = (Animal,)
spec (ConfigureIndexRequestSpec): [optional] # noqa: E501
deletion_protection (DeletionProtection): [optional] # noqa: E501
"""

_check_type = kwargs.pop("_check_type", True)
Expand All @@ -266,7 +267,6 @@ def __init__(self, spec, *args, **kwargs): # noqa: E501
self._configuration = _configuration
self._visited_composed_classes = _visited_composed_classes + (self.__class__,)

self.spec = spec
for var_name, var_value in kwargs.items():
if (
var_name not in self.attribute_map
Expand Down
Loading
Loading