Implement optional index tags in create_index and configure_index#426
Implement optional index tags in create_index and configure_index#426jhamon merged 3 commits intorelease-candidate/2025-01from
create_index and configure_index#426Conversation
create_index and configure_index
b67293b to
57cfac0
Compare
austin-denoble
left a comment
There was a problem hiding this comment.
LGTM
only nit is we may want to include a minimal amount of documentation about needing to use "" to delete / clear a tag
| replicas: Optional[int] = None, | ||
| pod_type: Optional[str] = None, | ||
| deletion_protection: Optional[Literal["enabled", "disabled"]] = None, | ||
| tags: Optional[Dict[str, str]] = None, |
There was a problem hiding this comment.
nit: Should the docstring in the interface be updated?
|
This might be an unnecessary question, but why do you have to merge existing tags? At least when I hit this endpoint in Node, existing tags are already merged (i.e. a configure req (containing a new tag) that is sent to an index w/existing tags already merges the new and existing tags). Previous to this PR, were tags being overwritten when updated when you hit this endpoint in Python? |
| :param: pod_type: the new pod_type for the index. To learn more about the | ||
| available pod types, please see [Understanding Indexes](https://docs.pinecone.io/docs/indexes) | ||
|
|
||
| :param: deletion_protection: If set to 'enabled', the index cannot be deleted. If 'disabled', the index can be deleted. |
There was a problem hiding this comment.
Nit: I'd add that enabled is the default here.
@aulorbe We're doing this to workaround some odd behavior in the generated python code. For example, when we first implemented Tags are a similar story. I was told the way people are supposed to delete things is by passing {"key": ""} to delete a key from tags. I wanted to test that and used merge to make explicit what is being sent in my tests rather than rely on any sort of implicit behavior around As for whether this is necessary or not in other SDKs, I think it really depends on how those language generators translate the spec into code. You'll have to do your own testing to make sure empty/default values are not doing unintended things when a user is only trying to modify other fields.
This is the first time we've had tags in the python SDK. So there's no precedent of doing it any other way. |
Problem
Want to expose kwargs for setting and modifying index tags
Solution
The usual. The generated bits backing this feature already got created when I generated off the
2025-01spec.I added a small amount of logic to handle merging new and existing tags when using configure.
Usage
Type of Change