-
Notifications
You must be signed in to change notification settings - Fork 14
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
V2 #233
Merged
Merged
V2 #233
Conversation
This file contains 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
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.
👍 Looks good to me! Reviewed everything up to 2eec271 in 1 minute and 55 seconds
More details
- Looked at
524
lines of code in8
files - Skipped
0
files when reviewing. - Skipped posting
10
drafted comments based on config settings.
1. src/zep_cloud/user/client.py:218
- Draft comment:
Docstring in 'delete' describes 'delete user by id'. For consistency and clarity, consider capitalizing ("Delete user by ID"). - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. src/zep_cloud/user/client.py:316
- Draft comment:
Consider refactoring repeated error handling blocks to reduce duplication (e.g. status code checks and JSON parsing). - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. src/zep_cloud/types/user_node_response.py:18
- Draft comment:
Docstrings and function names are auto-generated and look consistent. No issues found. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
4. src/zep_cloud/core/client_wrapper.py:20
- Draft comment:
Version in header 'X-Fern-SDK-Version' is updated to 2.4.0 – ensure this aligns with documentation. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
5. src/zep_cloud/graph/episode/client.py:217
- Draft comment:
Error handling for DELETE method includes explicit handling for 404. This is clear; consider adding comment documenting expected error scenarios. - Reason this comment was not posted:
Confidence changes required:30%
<= threshold50%
None
6. src/zep_cloud/user/client.py:96
- Draft comment:
There is a repeated pattern of calling _response.json() in both success and error branches. Consider caching the result in a variable to avoid redundant calls and potential issues if the response stream is consumed. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. src/zep_cloud/user/client.py:112
- Draft comment:
A similar error-handling pattern is repeated across methods (e.g., list_ordered, get, delete, update, get_facts, etc.). Consider refactoring the response parsing and error-handling logic into a shared helper to reduce duplication. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. src/zep_cloud/user/client.py:23
- Draft comment:
The global constant 'OMIT' is used as a default value for optional parameters. Consider adding an inline comment or documentation note to clarify its purpose for future maintainers. - Reason this comment was not posted:
Comment was on unchanged code.
9. src/zep_cloud/user/client.py:415
- Draft comment:
Docstrings include examples that use 'asyncio.run()'. Note that in environments with an existing running event loop (e.g., notebooks), this may cause issues. Consider noting alternative usage patterns for async calls. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
10. src/zep_cloud/user/client.py:349
- Draft comment:
The error-handling for API responses (checking status codes and raising specific errors) is consistent. Ensure that any additional status codes or edge cases defined in the API spec are handled appropriately, such as potential 401 or 403 errors. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_5wSfBPMWwYTkuUqU
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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.
Important
Add methods to handle user nodes and episode deletion, update version to 2.4.0, and enhance documentation.
get_node()
method toUserClient
andAsyncUserClient
inuser/client.py
to retrieve user nodes.delete()
method toEpisodeClient
andAsyncEpisodeClient
ingraph/episode/client.py
to delete episodes by UUID.UserNodeResponse
model intypes/user_node_response.py
.pyproject.toml
andclient_wrapper.py
.reference.md
with new methodsget_node()
anddelete()
for user and episode clients respectively.This description was created by
for 2eec271. It will automatically update as commits are pushed.