feat(agents-api): Remove auto_blob_store in favor of interceptor based system #977
Conversation
…d system Signed-off-by: Diwank Singh Tomer <[email protected]>
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 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. |
CI Failure Feedback 🧐(Checks updated until commit 76819e1)
✨ CI feedback usage guide:The CI feedback tool (
In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR: where Configuration options
See more information about the |
PR Reviewer Guide 🔍(Review updated until commit 76819e1)Here are some key observations to aid the review process:
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to e158f3a in 1 minute and 32 seconds
More details
- Looked at
5962lines of code in91files - Skipped
0files when reviewing. - Skipped posting
7drafted comments based on config settings.
1. agents-api/agents_api/activities/embed_docs.py:14
- Draft comment:
Ensure that the removal of@auto_blob_store(deep=True)is compensated by the new interceptor-based system to handle large data offloading. - Reason this comment was not posted:
Confidence changes required:50%
The removal of theauto_blob_storedecorator is consistent across multiple files. This change aligns with the PR's intent to replace it with an interceptor-based system. However, the decorator's removal should be verified to ensure that the new system handles the same functionality.
2. agents-api/agents_api/activities/execute_system.py:36
- Draft comment:
Verify that the new interceptor system handles the functionality previously managed byload_from_blob_store_if_remote. - Reason this comment was not posted:
Confidence changes required:50%
Theload_from_blob_store_if_remotefunction call is removed, which was responsible for loading data from a remote blob store. Ensure that the new interceptor system handles this functionality.
3. agents-api/agents_api/activities/task_steps/base_evaluate.py:62
- Draft comment:
Ensure that the removal of@auto_blob_store(deep=True)is consistent with the new interceptor-based system for data handling. - Reason this comment was not posted:
Confidence changes required:50%
Theauto_blob_storedecorator is removed from multiple functions. This change should be consistent with the new interceptor-based system to ensure data handling is still managed correctly.
4. agents-api/agents_api/activities/task_steps/cozo_query_step.py:10
- Draft comment:
Ensure that the removal of@auto_blob_store(deep=True)is consistent with the new interceptor-based system for data handling. - Reason this comment was not posted:
Confidence changes required:50%
Theauto_blob_storedecorator is removed from multiple functions. This change should be consistent with the new interceptor-based system to ensure data handling is still managed correctly.
5. agents-api/agents_api/activities/task_steps/evaluate_step.py:10
- Draft comment:
Ensure that the removal of@auto_blob_store(deep=True)is consistent with the new interceptor-based system for data handling. - Reason this comment was not posted:
Confidence changes required:50%
Theauto_blob_storedecorator is removed from multiple functions. This change should be consistent with the new interceptor-based system to ensure data handling is still managed correctly.
6. agents-api/agents_api/activities/task_steps/for_each_step.py:11
- Draft comment:
Ensure that the removal of@auto_blob_store(deep=True)is consistent with the new interceptor-based system for data handling. - Reason this comment was not posted:
Confidence changes required:50%
Theauto_blob_storedecorator is removed from multiple functions. This change should be consistent with the new interceptor-based system to ensure data handling is still managed correctly.
7. agents-api/agents_api/activities/task_steps/get_value_step.py:11
- Draft comment:
Ensure that the removal of@auto_blob_store(deep=True)is consistent with the new interceptor-based system for data handling. - Reason this comment was not posted:
Confidence changes required:50%
Theauto_blob_storedecorator is removed from multiple functions. This change should be consistent with the new interceptor-based system to ensure data handling is still managed correctly.
Workflow ID: wflow_AjDxfoccjl58OUog
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
PR Code Suggestions ✨Latest suggestions up to 76819e1
Previous suggestionsSuggestions up to commit e158f3a
|
Signed-off-by: Diwank Singh Tomer <[email protected]>
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on ca5f4e2 in 50 seconds
More details
- Looked at
53lines of code in4files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. agents-api/tests/fixtures.py:3
- Draft comment:
The import oftimeis unnecessary as it is not used in this file. Consider removing it to clean up the code. - Reason this comment was not posted:
Confidence changes required:50%
The import oftimeinagents-api/tests/fixtures.pyis unnecessary as it is not used anywhere in the file. Removing unused imports is a good practice to keep the code clean and efficient.
2. agents-api/agents_api/queries/sessions/create_session.py:11
- Draft comment:
The import ofResourceCreatedResponseis unnecessary as it is not used in this file. Consider removing it to clean up the code. - Reason this comment was not posted:
Confidence changes required:50%
The import ofResourceCreatedResponseinagents-api/agents_api/queries/sessions/create_session.pyis unnecessary as it is not used anywhere in the file. Removing unused imports is a good practice to keep the code clean and efficient.
3. agents-api/tests/test_session_queries.py:13
- Draft comment:
The import ofResourceCreatedResponseis unnecessary as it is not used in this file. Consider removing it to clean up the code. - Reason this comment was not posted:
Confidence changes required:50%
The import ofResourceCreatedResponseinagents-api/tests/test_session_queries.pyis unnecessary as it is not used anywhere in the file. Removing unused imports is a good practice to keep the code clean and efficient.
Workflow ID: wflow_4DsKSc8XWp0obflW
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
Persistent review updated to latest commit 76819e1 |
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 76819e1 in 1 minute and 23 seconds
More details
- Looked at
1358lines of code in37files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. agents-api/agents_api/routers/healthz/check_health.py:12
- Draft comment:
Consider using an asynchronous call forlist_agents_queryto avoid blocking behavior. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
This is a health check endpoint, so performance is not critical. The blocking nature of list_agents_query won't significantly impact the system since this endpoint is called infrequently. Making it async would be a nice-to-have optimization but not essential. The comment doesn't indicate a clear problem that needs fixing.
The comment could be valid if this is part of a larger async system where blocking calls are strictly avoided. The blocking call could potentially cause issues under high load.
For a health check endpoint, simplicity is more important than perfect async implementation. The blocking behavior is acceptable here given the endpoint's purpose and frequency of use.
Delete the comment as it suggests an optimization that isn't clearly necessary for this health check endpoint.
2. agents-api/agents_api/activities/sync_items_remote.py:14
- Draft comment:
Consider usingreturn_exceptions=Trueinasyncio.gatherto handle exceptions from individual coroutines without cancelling others. - Reason this comment was not posted:
Confidence changes required:50%
Insync_items_remote.py, thesave_inputs_remote_fnandload_inputs_remote_fnfunctions are usingasyncio.gatherwithout specifyingreturn_exceptions=True. This means if one coroutine raises an exception, the others will be cancelled. Consider whether this behavior is desired.
Workflow ID: wflow_0hHRxtRzosn0tTXv
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
User description
PR Type
Enhancement
Description
Changes walkthrough 📝
interceptors.py
Implement new interceptor-based blob store systemagents-api/agents_api/common/interceptors.py
remote.py
Simplify and improve remote object handlingagents-api/agents_api/common/protocol/remote.py
storage_handler.py
Remove storage handler in favor of interceptorsagents-api/agents_api/common/storage_handler.py
env.py
Update blob store configuration settingsagents-api/agents_api/env.py