-
Notifications
You must be signed in to change notification settings - Fork 215
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
Pinecone Native Integration #485
base: main
Are you sure you want to change the base?
Conversation
|
GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
---|---|---|---|---|---|
- | - | Generic High Entropy Secret | 4570e65 | agentops/llms/pinecone_test.py | View secret |
- | - | Generic High Entropy Secret | 5b0a5b1 | agentops/llms/pinecone_test.py | View secret |
- | - | Generic High Entropy Secret | 5b0a5b1 | agentops/llms/pinecone_test.py | View secret |
- | - | Generic High Entropy Secret | 4570e65 | agentops/llms/pinecone_test.py | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Amazing-- this is actually the first vectorDB integration PR we've seen so far. Will take a look soon @monami44. In the meantime-- can you resolve the merge conflict? |
Hey @areibman , I just resolved the merge conflict. Also here is a video-walkthrough I sent to Adam as I submitted the PR. Hit me up if you want anything else to be changed |
Super cool! @the-praxs and I can take a look |
provider.delete_assistant(pc, "test-assistant") | ||
print("Assistant deleted") | ||
|
||
os.remove("test_data.txt") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TL;DR tests should not write to physical file, but simulate doing so by writing memory.
Dangerous practice... in the rare but not impossible event this can lead to race conditions and surely host directory pollution, for which I suggest either standard pyfakefs or mktemp / tempfile
to avoid file collision at the very least.
Both provisioning and teardown belong to a fixture or a provisioner (if I remember correctly pyfakefs already handles it for you).
Added pyfakefs in PR #490
@monami44 I would request to do the following modifications -
|
Hey @areibman @the-praxs @teocns ! I've updated to match the new version, created example notebooks, applied LLMEvent to pinecone assistant chat function as well as did tests with temporary files as asked. Please tell me if anything else needs changes. Cheers, |
I will look in a while. Thanks a lot! |
Linting is failing so please resolve that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The most modifications are required in the Exception
block where a pprint
can help the user understand what's causing it.
Other changes are essential for code consistency and efficiency.
Also - please do the linting and resolve the errors with CI tests.
@dataclass | ||
class VectorEvent(Event): | ||
"""Event class for vector operations""" | ||
event_type: str = "action" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect type annotation and value. This should be EventType
with the value as EventType.VECTOR.value
.
Since that events is missing in the EventType
class, adding a VECTOR
enum will resolve the issu.
self._safe_record(session, event) | ||
return response | ||
except Exception as e: | ||
error_event = ErrorEvent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to use pprint
and print the Exception
in the console for the user to see what's causing it.
"query": kwargs.get("query") | ||
}) | ||
except Exception as e: | ||
details["error"] = str(e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as the other Exception
block, pprint
will look good!
self._safe_record(session, event) | ||
return response | ||
except Exception as e: | ||
error_event = ErrorEvent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as the other Exception
block - pprint
result = orig(*args, **kwargs) | ||
return self.handle_response(result, event_kwargs, init_timestamp, session=session) | ||
except Exception as e: | ||
# Create ActionEvent for the error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You know what I mean :)
response = pc_instance.assistant.create_assistant(**kwargs) | ||
return self._handle_assistant_response(response, "create_assistant", kwargs, init_timestamp, session) | ||
except Exception as e: | ||
error_event = ErrorEvent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:)
response = assistant.chat_completions(messages=message_objects, stream=stream, model=model) | ||
|
||
# Debug logging | ||
print(f"Debug - Raw response: {response}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace with logger.debug
to ensure consistent logging.
return completion_text | ||
|
||
except Exception as e: | ||
print(f"Debug - Exception in chat_completions: {str(e)}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace with logger.debug
to ensure consistent logging.
Pinecone Integration with Comprehensive Testing Suite
Overview
This PR introduces a comprehensive Pinecone integration along with extensive testing coverage across vector operations, RAG implementations, inference capabilities, and assistant functionalities.
Key Features
PineconeProvider
ClassTesting Suites
RAG Pipeline Test (
pinecone_rag_test.py
)Inference API Test (
pinecone_inference_test.py
)Assistant API Test (
pinecone_assistant_test.py
)Implementation Details
Testing Instructions
Ensure that the required environment variables are set:
PINECONE_API_KEY
OPENAI_API_KEY
(for RAG testing)Run individual test suites with the following commands:
Notes
Future Improvements
For more information, refer to the Pinecone Documentation.