-
Notifications
You must be signed in to change notification settings - Fork 905
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
feat(agents-api,integrations): Working integrations for tool-call step #521
Merged
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
Signed-off-by: Diwank Singh Tomer <[email protected]>
Signed-off-by: Diwank Singh Tomer <[email protected]>
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 f13f8dd in 33 seconds
More details
- Looked at
423
lines of code in12
files - Skipped
0
files when reviewing. - Skipped posting
6
drafted comments based on config settings.
1. agents-api/agents_api/activities/execute_integration.py:43
- Draft comment:
CatchingBaseException
is not recommended as it includes system-exiting exceptions. Consider catching more specific exceptions instead. - Reason this comment was not posted:
Comment was on unchanged code.
2. agents-api/agents_api/activities/execute_integration.py:50
- Draft comment:
mock_execute_integration
is set toexecute_integration
, which is redundant. Consider removingmock_execute_integration
and directly usingexecute_integration
. - Reason this comment was not posted:
Confidence changes required:50%
Themock_execute_integration
is set toexecute_integration
, which seems redundant sinceexecute_integration
is already defined. This could be simplified.
3. agents-api/agents_api/autogen/Tools.py:92
- Draft comment:
Ensure that the updated provider names (e.g.,dalle_image_generator
,duckduckgo_search
) are consistently used across all relevant files, including tests and other modules. - Reason this comment was not posted:
Confidence changes required:50%
TheIntegrationDef
class has been updated to use new provider names, but the changes should be consistent across all files where these literals are used.
4. agents-api/agents_api/clients/integrations.py:24
- Draft comment:
Consider adding exception handling for the HTTP request to manage potential errors, such as network issues or server errors. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is relevant to the changes made in the diff, as it pertains to the new function 'run_integration_service'. The suggestion to add exception handling is actionable and could improve the robustness of the code by handling network issues or server errors that 'raise_for_status()' does not cover. This aligns with the rule that suggests code quality refactors are good if they are actionable and clear.
The comment might be considered speculative since it assumes potential errors without evidence of them occurring. However, exception handling is a common practice to ensure robustness in network requests.
While the comment could be seen as speculative, it is a reasonable suggestion for improving code quality and robustness, which is a valid concern in network operations.
Keep the comment as it provides a clear and actionable suggestion to improve the code's robustness by adding exception handling for network requests.
5. agents-api/agents_api/env.py:74
- Draft comment:
Ensure thatINTEGRATION_SERVICE_URL
is set appropriately for different environments, as the defaulthttp://0.0.0.0:8000
might not be suitable for production. - Reason this comment was not posted:
Confidence changes required:50%
Theintegration_service_url
is set to a default value that might not be suitable for production environments.
6. agents-api/agents_api/workflows/task_execution/__init__.py:493
- Draft comment:
Ensure that the keysprovider
,setup
, andmethod
exist inintegration_spec.spec
to avoid potentialKeyError
. Consider using.get()
with default values. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_Ma7Pi9akdB8JoohC
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
creatorrr
added a commit
that referenced
this pull request
Sep 30, 2024
#521) - **fix(typespec,agents-api): Rename integration providers** - **feat(agents-api,integrations): Working integrations for tool-call step** <!-- ELLIPSIS_HIDDEN --> ---- > [!IMPORTANT] > Add integration service handling for tool-call steps in agents API, including provider renames and workflow updates. > > - **Integrations**: > - Implement `run_integration_service` in `clients/integrations.py` to handle integration service calls. > - Update `execute_integration` in `activities/execute_integration.py` to use `run_integration_service` for non-dummy providers. > - Add `INTEGRATION_SERVICE_URL` to `.env.example`, `env.py`, and `docker-compose.yml`. > - **Renames**: > - Rename integration providers in `Tools.py` and `scalars.tsp` (e.g., `dall-e` to `dalle_image_generator`). > - **Workflows**: > - Update `task_execution/__init__.py` to handle integration tool calls using `execute_integration`. > - **Tests**: > - Add `integration_example.yaml` for sample tasks. > - Add tests for integration tool calls in `test_execution_workflow.py`. > - Add `patch_integration_service` in `utils.py` for mocking integration service calls. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=julep-ai%2Fjulep&utm_source=github&utm_medium=referral)<sup> for f13f8dd. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN --> --------- Signed-off-by: Diwank Singh Tomer <[email protected]>
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 integration service handling for tool-call steps in agents API, including provider renames and workflow updates.
run_integration_service
inclients/integrations.py
to handle integration service calls.execute_integration
inactivities/execute_integration.py
to userun_integration_service
for non-dummy providers.INTEGRATION_SERVICE_URL
to.env.example
,env.py
, anddocker-compose.yml
.Tools.py
andscalars.tsp
(e.g.,dall-e
todalle_image_generator
).task_execution/__init__.py
to handle integration tool calls usingexecute_integration
.integration_example.yaml
for sample tasks.test_execution_workflow.py
.patch_integration_service
inutils.py
for mocking integration service calls.This description was created by for f13f8dd. It will automatically update as commits are pushed.