Add host validation and normalization to PineconeGRPC.Index() #573
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Resolved the host handling discrepancy between `Pinecone.Index()` and `PineconeGRPC.Index()`.
### Changes Made:
1. **Updated `pinecone/grpc/pinecone.py`**:
- Added imports for `normalize_host` from `pinecone.utils` and `check_realistic_host` from `pinecone.pinecone`
- Updated the `Index()` method to validate and normalize the host when provided, matching `Pinecone.Index()`:
- Validates the host using `check_realistic_host()` to ensure it's a valid host URL (not an index name)
- Normalizes the host using `normalize_host()` to ensure it has a protocol prefix (`https://` or `http://`)
2. **Added test coverage** in `tests/unit_grpc/test_grpc_index_initialization.py`:
- Added `test_invalid_host()` to verify that invalid hosts raise appropriate errors, matching the test for the regular Pinecone client
### Result:
Both `Pinecone.Index()` and `PineconeGRPC.Index()` now:
- Validate that provided hosts are valid (contain a "." or "localhost")
- Normalize hosts by adding "https://" prefix if no protocol is present
- Handle hosts consistently whether provided directly or fetched by index name
The implementation is now consistent between both clients, ensuring proper host validation and normalization in all cases.
## Summary Fixed a test failure found during CI checks: ### Issue Found: The test `test_config_passed_when_target_by_host_and_port` was failing because it used `host="myhost:4343"`, which doesn't pass the `check_realistic_host` validation (it requires a "." in the hostname or "localhost"). ### Fix Applied: Updated the test to use a valid hostname: `myhost.example.com:4343` instead of `myhost:4343`. This maintains the test's intent (testing hosts with port numbers) while using a valid hostname. ### Test Results: - All 8 tests in `test_grpc_index_initialization.py` pass - Linting passes - Type checking passes ### Current Status: - No PR exists for branch `vk/e239-resolve-descrepa` yet - All local checks pass - Code is ready for PR creation The changes are committed and ready. If you want, I can help create a PR or check for any other issues.
jhamon
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR resolves a discrepancy in host handling between the REST (
Pinecone) and gRPC (PineconeGRPC) clients. Previously,Pinecone.Index()validated and normalized host URLs, whilePineconeGRPC.Index()did not, leading to inconsistent behavior and potential user errors.Changes Made
Core Implementation
PineconeGRPC.Index()usingcheck_realistic_host()to ensure provided hosts are valid URLs (not index names)PineconeGRPC.Index()usingnormalize_host()to automatically add thehttps://protocol prefix when missingPinecone.Index()Implementation Details
The changes ensure that when a host is provided to
PineconeGRPC.Index():.(dot) or islocalhost, preventing common mistakes where users accidentally pass an index name as the host parameterhttps://orhttp://), ensuring consistent URL formattingThe implementation reuses existing utility functions:
check_realistic_host()- Validates that the host string appears to be a valid hostnamenormalize_host()- Ensures the host has a protocol prefixTest Coverage
test_invalid_host()to verify that invalid hosts (like"invalid"or"my-index") raise appropriateValueErrorexceptionstest_config_passed_when_target_by_host_and_portto use a valid hostname (myhost.example.com:4343instead ofmyhost:4343) to comply with validation requirementsWhy This Change Was Made
This change ensures consistency between both client implementations. Without host validation, users could accidentally pass index names as hosts, leading to cryptic DNS resolution errors later. The normalization ensures that hosts are properly formatted regardless of whether users include the protocol prefix, making the API more user-friendly and consistent.
Result
Both
Pinecone.Index()andPineconeGRPC.Index()now:.or arelocalhost)https://prefix if no protocol is presentThis PR was generated with Vibe Kanban and Cursor.