-
Notifications
You must be signed in to change notification settings - Fork 34
INTPYTHON-787 Added check for AZURE_OPENAI_ENDPOINT test_graphrag.py and cleaned up closing clients #232
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
INTPYTHON-787 Added check for AZURE_OPENAI_ENDPOINT test_graphrag.py and cleaned up closing clients #232
Conversation
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.
Pull Request Overview
This PR fixes a testing configuration issue where the GraphRAG tests were only checking for OpenAI API key but not Azure OpenAI endpoint, and implements better resource management by using context managers and explicit client closing.
- Updated test skip condition to check for both OPENAI_API_KEY and AZURE_OPENAI_ENDPOINT environment variables
- Replaced manual client creation/closing with context managers for better resource management
- Added explicit store.close() calls to properly clean up MongoDBGraphStore instances
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
LGTM pending a patch build
…and cleaned up closing clients (langchain-ai#232) [Issue Key](https://jira.mongodb.org/browse/{ISSUE_KEY}) <!-- What is this PR introducing? If context is already provided from the JIRA ticket, still place it in the Pull Request as you should not make the reviewer do digging for a basic summary. --> <!-- What changes did you make to the code? What new APIs (public or private) were added, removed, or edited to generate the desired outcome explained in the above summary? --> This is a small bugfix. When we switched from including OPENAI_API_KEY in CI secrets to using AZURE_OPENAI_ENDPOINTS, we missed this one. <!-- How did you test the code? If you added unit tests, you can say that. If you didn’t introduce unit tests, explain why. All code should be tested in some way – so please list what your validation strategy was. --> - Tests are run in github and evergreen. A passing patch build will be included. <!-- Do not delete the items provided on this checklist --> - [ na] Did you update the changelog (if necessary)? - [X] Is the intention of the code captured in relevant tests? - [na] If there are new TODOs, has a related JIRA ticket been created? - [X] Has a MongoDB Employee run [the patch build of this PR](https://github.com/mongodb-labs/ai-ml-pipeline-testing?tab=readme-ov-file#running-a-patch-build-of-a-given-pr)? - [x] Does the title of the PR reference a JIRA Ticket? - [x] Do you fully understand the implementation? (Would you be comfortable explaining how this code works to someone else?) - [x] Have you checked for spelling & grammar errors? - [x] Is all relevant documentation (README or docstring) updated? <!-- List any complex portion of code you believe needs particular scrutiny and explain why. --> The author had thought of updating MongoDBGraphStore to be a context manager. All this would do is close the client. We decided to leave it with a close method only.
…and cleaned up closing clients (langchain-ai#232) [Issue Key](https://jira.mongodb.org/browse/{ISSUE_KEY}) <!-- What is this PR introducing? If context is already provided from the JIRA ticket, still place it in the Pull Request as you should not make the reviewer do digging for a basic summary. --> <!-- What changes did you make to the code? What new APIs (public or private) were added, removed, or edited to generate the desired outcome explained in the above summary? --> This is a small bugfix. When we switched from including OPENAI_API_KEY in CI secrets to using AZURE_OPENAI_ENDPOINTS, we missed this one. <!-- How did you test the code? If you added unit tests, you can say that. If you didn’t introduce unit tests, explain why. All code should be tested in some way – so please list what your validation strategy was. --> - Tests are run in github and evergreen. A passing patch build will be included. <!-- Do not delete the items provided on this checklist --> - [ na] Did you update the changelog (if necessary)? - [X] Is the intention of the code captured in relevant tests? - [na] If there are new TODOs, has a related JIRA ticket been created? - [X] Has a MongoDB Employee run [the patch build of this PR](https://github.com/mongodb-labs/ai-ml-pipeline-testing?tab=readme-ov-file#running-a-patch-build-of-a-given-pr)? - [x] Does the title of the PR reference a JIRA Ticket? - [x] Do you fully understand the implementation? (Would you be comfortable explaining how this code works to someone else?) - [x] Have you checked for spelling & grammar errors? - [x] Is all relevant documentation (README or docstring) updated? <!-- List any complex portion of code you believe needs particular scrutiny and explain why. --> The author had thought of updating MongoDBGraphStore to be a context manager. All this would do is close the client. We decided to leave it with a close method only.
Issue Key
Summary
Changes in this PR
This is a small bugfix. When we switched from including OPENAI_API_KEY in CI secrets to using AZURE_OPENAI_ENDPOINTS, we missed this one.
Test Plan
Checklist
Checklist for Author
Checklist for Reviewer {@primary_reviewer}
Focus Areas for Reviewer (optional)
The author had thought of updating MongoDBGraphStore to be a context manager. All this would do is close the client. We decided to leave it with a close method only.