feat: integration/conn v1->v2 #1263
Draft
Conversation
This file contains hidden or 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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
![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 54eb407 in 1 minute and 15 seconds
More details
- Looked at
212lines of code in4files - Skipped
0files when reviewing. - Skipped posting
9drafted comments based on config settings.
1. python/composio/tools/toolset.py:1267
- Draft comment:
Using 'app_unique_key' (set from app_data.name) instead of the old 'app_id' as the primary identifier – please ensure documentation and downstream consumers are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to ensure documentation and downstream consumers are updated, which violates the rule against asking the author to ensure things are updated or tested. It does not provide a specific code suggestion or ask for a specific test to be written.
2. python/composio/tools/toolset.py:1276
- Draft comment:
initiate_connection – updated logic relies on integration V2 endpoints; ensure tests cover the new JSON payload and response structure. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the author to ensure that tests cover the new JSON payload and response structure. This falls under the rule of not asking the author to ensure that the change is tested. Therefore, this comment should be removed.
3. python/composio/tools/toolset.py:1299
- Draft comment:
Verifying auth_scheme against AUTH_SCHEMES is now crucial; ensure that the error message and upstream documentation reflect the permitted schemes. - 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.
4. python/composio/tools/toolset.py:1320
- Draft comment:
Ensure that _validate_auth_config is compatible with v2 integration requirements; update tests if needed. - 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.
5. composio/tools/toolset.py:340
- Draft comment:
Typo: Rename 'process_respone' to 'process_response' for clarity. - 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.
6. composio/tools/toolset.py:1258
- Draft comment:
Unused parameter 'force_new_integration' is documented with a pylint disable. Consider removing or documenting its no-op behavior. - 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. composio/tools/toolset.py:1267
- Draft comment:
Integration creation now uses 'app_unique_key' (from app_data.name) alongside app_id. Ensure the backend and clients expect this new contract. - 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. composio/client/endpoints.py:142
- Draft comment:
Ensure that the new v2 endpoints (for integrations, actions, and triggers) are correctly configured and tested. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
9. composio/tools/toolset.py:1716
- Draft comment:
Consider supporting additional types (e.g. datetime) in _serialize_execute_params if they are expected in action parameters. - Reason this comment was not posted:
Confidence changes required:40%<= threshold50%
None
Workflow ID: wflow_XRpGxnAKCsWDdgsA
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Code Review SummaryOverall AssessmentThe changes for migrating from v1 to v2 API are generally well-structured, but there are a few areas that need attention: Key Concerns
Recommendations
Code Quality: 7/10
|
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 8bbfe42 in 1 minute and 35 seconds
More details
- Looked at
115lines of code in3files - Skipped
0files when reviewing. - Skipped posting
8drafted comments based on config settings.
1. python/composio/tools/toolset.py:340
- Draft comment:
Typo: Rename 'process_respone' to 'process_response' to improve clarity. - 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. python/composio/tools/toolset.py:1880
- Draft comment:
Consider adding a brief delay between retries in the execute_action retry loop to avoid potential tight loops. - 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. python/composio/tools/toolset.py:237
- Draft comment:
The use of the '@' operator in 'return action @ self[action]' is non-standard; ensure its behavior is clearly documented. - 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.
4. python/composio/client/collections.py:257
- Draft comment:
Updated payload structure: the JSON now nests 'config' and 'connection' objects. Verify that the backend v2 endpoint accepts these new field names. - 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.
5. python/composio/tools/toolset.py:1258
- Draft comment:
Integration creation now uses the v2 endpoint and omits the app_id parameter. Confirm that using app_unique_key (possibly lowercased) meets the new backend requirements. - 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.
6. python/composio/tools/toolset.py:1340
- Draft comment:
In initiate_connection, when required connected_account_params are missing, the error message could include more context (e.g. expected integration parameters) to aid debugging. - 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. python/composio/tools/toolset.py:340
- Draft comment:
Typo: Method name 'process_respone' should be 'process_response'. - 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. python/composio/tools/toolset.py:1110
- Draft comment:
Typo in error message: 'available_schems' should be 'available_schemes'. - 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_FMg23pgljNPQxNVS
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
angrybayblade
approved these changes
Feb 10, 2025
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 64ea12d in 1 minute and 12 seconds
More details
- Looked at
34lines of code in1files - Skipped
1files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. python/composio/client/__init__.py:424
- Draft comment:
Updating the type and default value of 'force_new_integration' from bool to Optional[bool] changes its behavior. Consider updating the function's docstring to inform users that this parameter is deprecated and its default is now None. - 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. python/composio/client/__init__.py:438
- Draft comment:
When warning about deprecation, consider specifying a warning category (e.g., DeprecationWarning or FutureWarning) and setting stacklevel=2 to better indicate the source of the call. - 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. python/composio/client/__init__.py:424
- Draft comment:
Changed 'force_new_integration' from bool with default False to Optional[bool] = None. Consider updating the function docstring to note its deprecation clearly. - 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_ywAD72eZ2EBCKLUV
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on b61ddb2 in 28 seconds
More details
- Looked at
15lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. python/composio/client/__init__.py:456
- Draft comment:
Formatting improvement: splitting the error message across multiple lines improves readability. Ensure consistency with the project's style guide. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
2. python/composio/client/__init__.py:456
- Draft comment:
This change only reformats the raise statement for improved readability; no functional changes. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
Workflow ID: wflow_WPzVOFb5Ybm9HYvs
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 2e7f54e in 2 minutes and 35 seconds
More details
- Looked at
1537lines of code in44files - Skipped
1files when reviewing. - Skipped posting
44drafted comments based on config settings.
1. .github/workflows/release.yaml:66
- Draft comment:
Added new plugin names in the plugins list; please verify that these plugin identifiers are correct and match the expected configuration for release builds. Also, double-check that modifying the environment variable for PATH using export is effective within the workflow step. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to verify and double-check things, which violates the rules. It doesn't provide a specific suggestion or point out a specific issue with the code.
2. docs/framework/agno.mdx:12
- Draft comment:
Renamed from 'Phidata' to 'Agno' and updated instructions. Confirm that all references (e.g., package names and example code) are updated consistently. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to confirm that all references have been updated consistently. This falls under the rule of not asking the author to double-check things. The comment does not provide a specific suggestion or point out a specific issue, so it should be removed.
3. docs/framework/gemini.mdx:22
- Draft comment:
The Gemini guide now provides clear steps for initializing a Google client and configuring tools. Ensure that the demo code examples accurately match the live client SDK methods. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative and does not provide a specific code suggestion or request for clarification. It also asks the PR author to ensure something, which is against the rules.
4. docs/framework/groq.mdx:18
- Draft comment:
The Groq guide appears largely unchanged except for minor copy edits. Verify that code snippets and command examples (e.g., for installing composio-langchain) are accurate. - 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.
5. docs/framework/ibm.mdx:60
- Draft comment:
The IBM integration guide (video and code) looks good and the instructions are clear. Validate that the IBM agent setup code correctly enforces required inputs (e.g. URL and Project ID) and that error messages have been improved. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment starts with a purely informative statement about the IBM integration guide, which is not allowed. It then asks the PR author to validate certain aspects of the code, which is also not allowed as per the rules. The comment does not provide a specific code suggestion or ask for a specific test to be written.
6. docs/mint.json:150
- Draft comment:
The updated mint.json now includes new frameworks (gemini and ibm) and renames phidata to agno. Confirm that documentation section links mirror updated file names. - 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. js/examples/deep_researcher/demo.mjs:45
- Draft comment:
The Deep Researcher agent example includes a custom CLI flow with spinner animations. Recommend verifying that usage of environment variables (e.g. COMPOSIO_API_KEY) is secure and error handling in promise rejections (process.exit on error) is acceptable. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =30%<= threshold50%
The comment suggests verifying the security of environment variables and error handling practices. It indirectly asks the author to ensure these practices are correct, which violates the rule against asking the author to confirm or ensure behavior. However, it does not provide a specific suggestion or ask for a specific change, making it less actionable.
8. python/composio/__version__.py:1
- Draft comment:
Version bump from 0.7.0 to 0.7.1 is consistent across packages. Ensure that version changes are synchronized in all dependent tools. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is related to dependency version changes, which is not allowed according to the rules. It also asks the author to ensure synchronization across tools, which is similar to asking for confirmation or verification, which is not allowed.
9. python/composio/client/__init__.py:454
- Draft comment:
Updated error messages to reference AUTH_SCHEME_WITH_INITIATE properly. This change should be validated to ensure there is no conflict with existing authentication modes. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to validate the change to ensure there is no conflict with existing authentication modes. This falls under the rule of not asking the author to ensure the behavior is intended or to double-check things. Therefore, this comment should be removed.
10. python/composio/client/collections.py:42
- Draft comment:
The new definitions for ALL_AUTH_SCHEMES and AUTH_SCHEME_WITH_INITIATE look appropriate. Please check that TYPE_CHECKING and literal definitions remain consistent with actual usage. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to check consistency, which violates the rule against asking the author to double-check things. It does not provide a specific suggestion or point out a specific issue.
11. python/composio/tools/toolset.py:1272
- Draft comment:
Updated initiate_connection method to validate auth_scheme using AUTH_SCHEME_WITH_INITIATE. Confirm that the _validate_no_auth_scheme helper is properly called and that tests cover scenarios with 'NO_AUTH'. - 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.
12. python/dockerfiles/Dockerfile:22
- Draft comment:
The Dockerfile now installs composio-core version 0.7.1. Ensure that all dependencies (e.g., fastapi, playwright, uvicorn) are compatible with this core version. - Reason this comment was not posted:
Marked as duplicate.
13. python/examples/advanced_agents/domain_deep_research/readme.md:24
- Draft comment:
Updated command to run main.py has correct path. Confirm that instructions align with the file structure and that the 'domain_deep_research' directory accurately reflects updated naming. - 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.
14. python/examples/advanced_agents/domain_deep_research/setup.sh:19
- Draft comment:
The setup.sh file creates a venv named 'domain_researcher' and executes login and addition of integrations. Verify that commands (e.g. 'composio add exa' and 'composio add googledocs') match the intended integrations. - 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.
15. python/plugins/agno/README.md:13
- Draft comment:
Documentation for the Agno plugin appears clear. Ensure that installation instructions and sample commands (composio add github, etc.) reflect the latest SDK behavior. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative and asks the PR author to ensure that documentation reflects the latest SDK behavior. It doesn't provide a specific code suggestion or point out a specific issue in the code.
16. python/plugins/agno/agno_demo.py:6
- Draft comment:
The Agno demo example is concise and tests a simple tool (gmail profile). Check that the action name 'GMAIL_GET_PROFILE' is valid and that error handling (if profile retrieval fails) is covered in tests. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =40%<= threshold50%
The comment is asking the PR author to check the validity of an action name and ensure error handling is covered in tests. This violates the rule against asking the author to confirm or ensure things. However, it does suggest a specific area for testing, which is allowed.
17. python/plugins/agno/composio_agno/__init__.py:1
- Draft comment:
Public API re-exports are correctly updated, including WorkspaceType and action. No issues. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states that the public API re-exports are correctly updated, which is not necessary for the PR author to know from a review perspective.
18. python/plugins/agno/composio_agno/toolset.py:116
- Draft comment:
The Agno-specific toolset wraps composio tools as Agno Toolkit objects. Verify that the conversion and function wrapping, including validate_call usage, provide correct parameter binding. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to verify the conversion and function wrapping, which violates the rule against asking the author to ensure behavior is intended or tested. It doesn't provide a specific suggestion or point out a specific issue.
19. python/plugins/agno/setup.py:10
- Draft comment:
Setup script for Agno plugin specifies dependencies correctly. Ensure versions for composio_core and agno packages are compatible. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative and asks the PR author to ensure compatibility of versions, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue.
20. python/plugins/autogen/setup.py:10
- Draft comment:
Setup script for Autogen plugin updated to version 0.7.1. Confirm that dependency versions for pyautogen, flaml, and autogen_agentchat are compatible. - Reason this comment was not posted:
Marked as duplicate.
21. python/plugins/camel/setup.py:10
- Draft comment:
Camel plugin setup, version bumped to 0.7.1. Validate that the camel-ai version specifier meets current SDK requirements. - Reason this comment was not posted:
Marked as duplicate.
22. python/plugins/claude/setup.py:10
- Draft comment:
Claude plugin setup calls for composio_openai and anthropic. Confirm that the dependency versions support the intended functionality. - Reason this comment was not posted:
Marked as duplicate.
23. python/plugins/crew_ai/setup.py:10
- Draft comment:
CrewAI plugin setup is updated to version 0.7.1, with dependency on composio_langchain. Ensure tests cover connectivity with CrewAI agents. - Reason this comment was not posted:
Marked as duplicate.
24. python/plugins/gemini/README.md:1
- Draft comment:
Gemini plugin documentation includes a detailed integration guide. Check that all instructions (e.g., token usage and client configuration) are up-to-date with Google Genai SDK. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative and asks the PR author to check if the documentation is up-to-date, which violates the rule against asking the author to ensure things are correct or up-to-date.
25. python/plugins/gemini/composio_gemini/__init__.py:1
- Draft comment:
Exports for Gemini toolset are clean and complete. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It does not align with the rules for good comments, as it does not ask for clarification, suggest improvements, or identify potential issues.
26. python/plugins/gemini/composio_gemini/toolset.py:11
- Draft comment:
Gemini toolset wrapping function correctly builds a callable function for executing actions. Confirm that the creation of the function signature and annotations via get_signature_format_from_schema_params meets expectations. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to confirm that the creation of the function signature and annotations meets expectations. This falls under asking the author to confirm their intention or to ensure the behavior is intended, which violates the rules.
27. python/plugins/gemini/demo.py:1
- Draft comment:
Gemini demo sample provided displays a clean integration with Google Genai SDK. Validate that paradigms for creating a client and sending messages are correct. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to validate that the paradigms for creating a client and sending messages are correct. This falls under the category of asking the author to ensure the behavior is intended or to double-check things, which is against the rules.
28. python/plugins/gemini/setup.py:1
- Draft comment:
Gemini plugin setup script updated to version 0.7.1; dependency on google-genai should be verified. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is about verifying a dependency change, which is not allowed according to the rules. It doesn't provide a specific suggestion or ask for a test to be written. Therefore, it should be removed.
29. python/plugins/google/setup.py:10
- Draft comment:
Google plugin setup updated; dependency on google-cloud-aiplatform is specified. Ensure testing against actual cloud API responses. - Reason this comment was not posted:
Marked as duplicate.
30. python/plugins/griptape/setup.py:10
- Draft comment:
Griptape plugin setup updated to version 0.7.1. Validate that necessary dependencies as provided in requirements are tested. - Reason this comment was not posted:
Marked as duplicate.
31. python/plugins/julep/setup.py:10
- Draft comment:
Julep plugin setup updated to version 0.7.1. Confirm that dependency versions (such as composio_openai and julep) are aligned. - Reason this comment was not posted:
Marked as duplicate.
32. python/plugins/langchain/setup.py:10
- Draft comment:
Langchain plugin setup reflects new version and dependencies; check that packages like langchain-openai and langchainhub are compatible with latest features. - Reason this comment was not posted:
Marked as duplicate.
33. python/plugins/langgraph/setup.py:10
- Draft comment:
Langgraph plugin setup updated; dependency on langgraph is included. Verify that the plugin integration meets the expected workflow. - Reason this comment was not posted:
Marked as duplicate.
34. python/plugins/llamaindex/setup.py:10
- Draft comment:
LlamaIndex plugin setup updated to version 0.7.1; ensure dependency version for llama_index is as required. - Reason this comment was not posted:
Marked as duplicate.
35. python/plugins/lyzr/setup.py:10
- Draft comment:
Lyzr plugin setup updated to version 0.7.1; verify that all dependencies (lyzr-automata, pydantic, langchain) are compatible with the latest core. - Reason this comment was not posted:
Marked as duplicate.
36. python/plugins/openai/setup.py:10
- Draft comment:
OpenAI plugin setup updated; dependency on openai is present. Ensure that SDK methods and function call formatting align with OpenAI function-calling requirements. - Reason this comment was not posted:
Marked as duplicate.
37. python/plugins/phidata/composio_phidata/toolset.py:35
- Draft comment:
Phidata toolset is marked as deprecated in favor of composio_agno. Ensure that a deprecation warning is clear and that existing users transition accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative and suggests ensuring a deprecation warning is clear and that users transition accordingly. It doesn't provide a specific code suggestion or ask for a specific test to be written. It violates the rule against making purely informative comments.
38. python/plugins/phidata/setup.py:10
- Draft comment:
PhiData plugin setup updated to version 0.7.1; note that this package is deprecated. Ensure that documentation clearly instructs users to migrate to composio_agno. - Reason this comment was not posted:
Marked as duplicate.
39. python/plugins/praisonai/setup.py:10
- Draft comment:
PraisonAI plugin setup updated to version 0.7.1; dependency on PraisonAI version is specified. Check that string formatting in descriptions is correct. - Reason this comment was not posted:
Marked as duplicate.
40. python/plugins/pydanticai/setup.py:10
- Draft comment:
Pydantic AI plugin setup updated to version 0.7.1; ensure that dependency on pydantic-ai is correct and that version constraints align with composio_core. - Reason this comment was not posted:
Marked as duplicate.
41. python/setup.py:90
- Draft comment:
Core package setup updated consistently to version 0.7.1. Confirm that scan_for_package_data properly finds required package files and that extras (all, tools, e2b, etc.) are correctly defined. - Reason this comment was not posted:
Marked as duplicate.
42. python/swe/setup.py:36
- Draft comment:
SWE kit setup updated to version 0.4.1. Verify that entry point is defined correctly and all extra dependencies (for langgraph integration) are fully tested. - Reason this comment was not posted:
Marked as duplicate.
43. python/composio/tools/toolset.py:1295
- Draft comment:
In the initiate_connection method, the code checks the auth_scheme by calling _validate_no_auth_scheme and then immediately raises an error if auth_scheme isn’t in AUTH_SCHEME_WITH_INITIATE. Consider refactoring this to streamline error handling and avoid redundant checks. - 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.
44. python/dockerfiles/Dockerfile:19
- Draft comment:
The RUN command uses 'export PATH=$PATH:$(pwd)/.composio/venv/bin', which only affects the current layer. Use the ENV instruction to ensure the PATH is updated persistently. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_VwwvZpuVn1OVo3zc
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on a0d6dce in 1 minute and 38 seconds
More details
- Looked at
19lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. python/composio/client/__init__.py:449
- Draft comment:
Switching from ComposioClientError to InvalidParams is fine. Consider updating docstrings to list valid auth_mode values (AUTH_SCHEME_WITH_INITIATE) for clarity. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
2. python/composio/client/__init__.py:454
- Draft comment:
The added comment for mypy casting is useful. Optionally, include a reference to the specific mypy error to clarify the workaround. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
3. python/composio/client/__init__.py:449
- Draft comment:
Using InvalidParams instead of ComposioClientError here clarifies that this is a parameter validation issue. Ensure downstream code/documentation aligns with this change. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =50%<= threshold50%
The comment suggests using a specific error type to clarify the nature of the error, which is a valid code suggestion. However, it also asks to ensure that downstream code/documentation aligns with this change, which violates the rule against asking the author to ensure things are aligned or tested.
4. python/composio/client/__init__.py:454
- Draft comment:
The added comment explaining the need for t.cast to appease mypy is helpful. Consider if a runtime check against AUTH_SCHEME_WITH_INITIATE might further safeguard against invalid auth_mode values. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_uWxZKLZzjwowwLi2
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
This reverts commit edab51c45e5eed07b204b94b53d8c6838ca67113.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on b149777 in 1 minute and 2 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
5drafted comments based on config settings.
1. python/tests/test_tools/test_toolset.py:24
- Draft comment:
Good clean-up: Removed the unused import 'E2E' from tests.conftest. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
2. python/tests/test_tools/test_toolset.py:25
- Draft comment:
Good removal of the unused importE2E; keeping the test file clean and avoiding unused dependencies. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
3. python/tests/test_tools/test_toolset.py:238
- Draft comment:
Typo: In the definition of weird_postprocessor, the parameter 'reponse' should be spelled 'response'. - 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.
4. python/tests/test_tools/test_toolset.py:362
- Draft comment:
Typo: In the test_execute_action function, the key 'successfull' appears to be misspelled. Consider changing it to 'successful' for consistency. - 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.
5. python/tests/test_tools/test_toolset.py:535
- Draft comment:
Typo: The class names 'SomeToolsetExtention' and 'SomeOtherToolsetExtention' use an incorrect spelling. Consider renaming them to 'SomeToolsetExtension' and 'SomeOtherToolsetExtension' respectively. - 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_ASRiOyusQa74iE52
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
composio/python/composio/tools/toolset.py
Line 1277 in 8b54470
Update this as well
| @@ -26,6 +26,7 @@ def initialize_peopledatalabs(entity_id: str, toolset: ComposioToolSet, peopleda | |||
| connection_request = entity.initiate_connection( | |||
| app_name=App.PEOPLEDATALABS, | |||
| auth_mode="API_KEY", | |||
| use_composio_auth=False, | |||
There was a problem hiding this comment.
Why was this changed?
There was a problem hiding this comment.
Reverted this change, was added as part of debugging.
| @classmethod | ||
| def teardown_class(cls) -> None: | ||
| for integration_id in cls.integration_ids: | ||
| cls.toolset.client.integrations.remove(id=integration_id) |
There was a problem hiding this comment.
Check if this was successful or not, if not fail the test
There was a problem hiding this comment.
Added this check, by modifying the remove method and checking the status code returned.
python/composio/tools/toolset.py
Outdated
| @@ -1254,17 +1254,16 @@ def create_integration( | |||
| auth_mode: t.Optional[AuthSchemeType] = None, | |||
| auth_config: t.Optional[t.Dict[str, t.Any]] = None, | |||
| use_composio_oauth_app: bool = True, | |||
| force_new_integration: bool = False, | |||
| force_new_integration: bool = False, # pylint: disable=unused-argument | |||
There was a problem hiding this comment.
Deprecation warning
python/composio/client/__init__.py
Outdated
Comment on lines
449
to
451
| if auth_mode is None: | ||
| raise InvalidParams( | ||
| f"'auth_mode' should be one of {AUTH_SCHEME_WITH_INITIATE}" |
There was a problem hiding this comment.
This is very confusing message/check, if auth_mode is required why is an optional parameter?
There was a problem hiding this comment.
I have made auth_mode a required parameter, since according to the logic users must be passing it so don't have to worry about backwards compatibility. And have improved the error message returned.
Comment on lines
420
to
422
| app_name: t.Union[str, App], | ||
| auth_mode: t.Optional[str] = None, | ||
| auth_mode: str, | ||
| auth_config: t.Optional[t.Dict[str, t.Any]] = None, |
There was a problem hiding this comment.
Removing auth_mode default value of None but not updating all call sites could cause runtime errors. Either keep the default or update all callers.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| app_name: t.Union[str, App], | |
| auth_mode: t.Optional[str] = None, | |
| auth_mode: str, | |
| auth_config: t.Optional[t.Dict[str, t.Any]] = None, | |
| app_name: t.Union[str, App], | |
| auth_mode: t.Optional[str] = None, | |
| auth_config: t.Optional[t.Dict[str, t.Any]] = None, |
Comment on lines
+450
to
453
| if auth_mode not in AUTH_SCHEME_WITH_INITIATE: | ||
| raise InvalidParams( | ||
| f"'auth_mode' should be one of the following strings {AUTH_SCHEME_WITH_INITIATE}" | ||
| ) |
There was a problem hiding this comment.
Logic change from checking if auth_mode is None to if auth_mode not in AUTH_SCHEME_WITH_INITIATE inverts the error condition and may silently accept invalid values.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| if auth_mode not in AUTH_SCHEME_WITH_INITIATE: | |
| raise InvalidParams( | |
| f"'auth_mode' should be one of the following strings {AUTH_SCHEME_WITH_INITIATE}" | |
| ) | |
| if auth_mode is None or auth_mode not in AUTH_SCHEME_WITH_INITIATE: | |
| raise InvalidParams( | |
| f"'auth_mode' should be one of the following strings {AUTH_SCHEME_WITH_INITIATE}" | |
| ) |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 1741af8 in 4 minutes and 24 seconds
More details
- Looked at
109lines of code in5files - Skipped
0files when reviewing. - Skipped posting
21drafted comments based on config settings.
1. python/examples/miscellaneous/multi_entity.py:4
- Draft comment:
Remove unused import: 'from fastapi import params' is not used in this file. - 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. python/tests/test_tools/test_toolset.py:92
- Draft comment:
Ensure that the error message in the expected exception for 'test_uninitialize_app' is updated if the message text changes in the SDK for consistency. - 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. python/tests/test_tools/test_toolset.py:120
- Draft comment:
Consider using more explicit assertion over checking log content in 'test_validate_tools_app/action'. - 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.
4. python/tests/test_tools/test_toolset.py:292
- Draft comment:
It might be beneficial to add comments clarifying the purpose and context of the custom auth tests for local and runtime tools. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
5. python/examples/miscellaneous/multi_entity.py:19
- Draft comment:
Avoid catching a bare Exception when trying to get a connection. Consider catching a more specific exception to prevent masking unrelated 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.
6. python/examples/miscellaneous/multi_entity.py:47
- Draft comment:
Using print statements for error reporting is not ideal; consider using a logging framework for better consistency and control over log output. - 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. python/tools/toolset.py:327
- Draft comment:
Typo: The method name 'process_respone' appears to be misspelled. Consider renaming it to 'process_response' to improve clarity and consistency. - 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. python/tools/toolset.py:1890
- Draft comment:
Consider implementing exponential backoff between retries in 'execute_action' to avoid rapid-fire retry loops in case of transient failures. - 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.
9. python/tests/test_tools/test_toolset.py:60
- Draft comment:
When checking connected accounts (e.g. account.appName == 'gmail'), consider normalizing case so that mismatches due to letter casing are avoided. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
10. python/tools/toolset.py:1290
- Draft comment:
In 'initiate_connection', after creating a new integration, caching or reusing the integration_id might help avoid duplicate creations if the call is repeated. Consider reviewing if redundant integrations are created. - Reason this comment was not posted:
Confidence changes required:40%<= threshold50%
None
11. python/composio/client/collections.py:315
- Draft comment:
Typo in the docstring of AppAuthScheme: 'App authenticatio scheme.' should be 'App authentication scheme.' - 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.
12. python/composio/client/collections.py:487
- Draft comment:
Typo in FileType field description: 'indetify' should be 'identify' ("File name, contains extension to indetify the file type"). - 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.
13. python/composio/client/collections.py:519
- Draft comment:
Typo in _ChunkedTriggerEventData docstring: 'Cunked trigger event data model.' should be 'Chunked trigger event data model.' - 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.
14. python/composio/client/collections.py:658
- Draft comment:
Typo in TriggerSubscription.callback docstring: 'Register a trigger callaback.' should be 'Register a trigger callback.' - 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.
15. python/composio/client/collections.py:1084
- Draft comment:
Typo in SearchResultTask field description: 'Descrption of the subtask.' should be 'Description of the subtask.' - 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.
16. python/composio/tools/toolset.py:1180
- Draft comment:
Typo found: 'suppossed' should be corrected to 'supposed' in the docstring describing expected parameters for the user. - 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.
17. python/composio/tools/toolset.py:1208
- Draft comment:
Typographical error: 'integratuib' should be corrected to 'integration' in the docstring that explains creation of an integration. - 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.
18. python/composio/tools/toolset.py:1507
- Draft comment:
Typographical error: 'Heres and example' should be corrected to "Here's an example" in the docstring for the request pre-processor example. - 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.
19. python/tests/test_tools/test_toolset.py:362
- Draft comment:
Typographical error: The key 'successfull' appears to have an extra 'l'. If the intended word is 'successful', please correct it. - 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.
20. python/tests/test_tools/test_toolset.py:238
- Draft comment:
Typographical error: The parameter name 'reponse' is likely intended to be 'response'. - 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.
21. python/tests/test_tools/test_toolset.py:535
- Draft comment:
Typographical error: 'SomeToolsetExtention' (and similarly 'SomeOtherToolsetExtention') should be spelled 'SomeToolsetExtension' for consistency and clarity. - 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_Uj7Yp14GsW4vkr4V
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
This reverts commit 498348966776b06e7a94aff5313de27821e2b65e.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 594dc06 in 1 minute and 32 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. python/tests/test_tools/test_toolset.py:645
- Draft comment:
Usepytest.fail('message')directly.pytest.fail()already raises an exception, so there's no need to useraise. - Reason this comment was not posted:
Marked as duplicate.
2. python/tests/test_tools/test_toolset.py:362
- Draft comment:
Typographical error: The key 'successfull' in the assertion should be corrected to 'successful'. - 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_DwprKQUdz4gd9VhV
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| for integration_id in cls.integration_ids: | ||
| status = cls.toolset.client.integrations.remove(id=integration_id) | ||
| if status != 204: | ||
| raise pytest.fail(reason="Unable to perform teardown successfully.") |
There was a problem hiding this comment.
Use pytest.fail() directly rather than raising it; pytest.fail() itself raises an exception.
Suggested change
| raise pytest.fail(reason="Unable to perform teardown successfully.") | |
| pytest.fail(reason="Unable to perform teardown successfully.") |
Comment on lines
+1314
to
+1325
| if app is not None: | ||
| app_name = app if isinstance(app, str) else app.name | ||
|
|
||
| timestamp = datetime.now().strftime("%Y%m%d%H%M%S") | ||
| return self.client.connected_accounts.initiate( | ||
| integration_id=integration_id, | ||
| entity_id=entity_id or self.entity_id, | ||
| params=connected_account_params, | ||
| labels=labels, | ||
| redirect_url=redirect_url, | ||
| app_unique_key=app_name, # pylint: disable=E0606 | ||
| name=f"{app_name}_{timestamp}", |
There was a problem hiding this comment.
Using undefined variable app_name in initiate() call when app is None. Need to initialize app_name only when app is provided.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| if app is not None: | |
| app_name = app if isinstance(app, str) else app.name | |
| timestamp = datetime.now().strftime("%Y%m%d%H%M%S") | |
| return self.client.connected_accounts.initiate( | |
| integration_id=integration_id, | |
| entity_id=entity_id or self.entity_id, | |
| params=connected_account_params, | |
| labels=labels, | |
| redirect_url=redirect_url, | |
| app_unique_key=app_name, # pylint: disable=E0606 | |
| name=f"{app_name}_{timestamp}", | |
| app_name = None | |
| if app is not None: | |
| app_name = app if isinstance(app, str) else app.name | |
| timestamp = datetime.now().strftime("%Y%m%d%H%M%S") | |
| return self.client.connected_accounts.initiate( | |
| integration_id=integration_id, | |
| entity_id=entity_id or self.entity_id, | |
| params=connected_account_params, | |
| labels=labels, | |
| redirect_url=redirect_url, | |
| app_unique_key=app_name, | |
| name=f"{app_name}_{timestamp}" if app_name else timestamp, |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on e57e43e in 1 minute and 59 seconds
More details
- Looked at
88lines of code in1files - Skipped
0files when reviewing. - Skipped posting
9drafted comments based on config settings.
1. python/composio/tools/toolset.py:1314
- Draft comment:
Removed auto integration creation & param validation logic. Ensure this simplification is intended for v2 migration. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
The comment is pointing out a significant simplification of the code, which could be a breaking change. However, it's phrased as "Ensure this simplification is intended" which violates our rule about not asking the PR author to verify things. The comment is not actionable - it's just asking for confirmation rather than suggesting a specific change. The comment also mentions "v2 migration" but there's no clear evidence in the code that this is related to a v2 migration.
The code change is significant and could affect functionality - maybe we're being too quick to dismiss a comment about such a large change?
While the change is significant, our rules are clear that we should not ask authors to verify their intentions. If there was a real issue with the change, the comment should point out the specific problem rather than just asking for confirmation.
The comment should be deleted because it violates our rule about not asking authors to verify their intentions. If there are specific concerns about the removed functionality, they should be raised directly rather than asking for confirmation.
2. python/composio/tools/toolset.py:1317
- Draft comment:
Potential issue: if 'app' is None, 'app_name' is undefined resulting in a NameError. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. python/composio/tools/toolset.py:1314
- Draft comment:
Potential bug: if 'app' is None the variable 'app_name' is never defined but is later used (in app_unique_key and name). Consider handling the case when app is None (e.g. by raising an error or providing a default). - Reason this comment was not posted:
Comment looked like it was already resolved.
4. python/composio/tools/toolset.py:1310
- Draft comment:
Noticeable behavioral change: the removed block for auto-creation of integrations and validation of connected_account_params has been dropped. Ensure this change is intentional and that the docstrings/documentation are updated to reflect the new v2 behavior. - 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.
5. python/composio/tools/toolset.py:328
- Draft comment:
Typo detected: The method name 'process_respone' should be renamed to 'process_response' to fix the spelling error. - 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.
6. python/composio/tools/toolset.py:1180
- Draft comment:
Typographical error: In the docstring of get_expected_params_for_user, 'suppossed' should be corrected to 'supposed'. - 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. python/composio/tools/toolset.py:1662
- Draft comment:
Typographical error: In the docstring of _try_get_github_access_token_for_current_entity, 'entiry' should be corrected to 'entity'. - 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. python/composio/tools/toolset.py:1114
- Draft comment:
Typographical error: In the error message of get_auth_scheme_for_app, 'available_schems' should be corrected to 'available_schemes'. - 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.
9. python/composio/tools/toolset.py:1443
- Draft comment:
Typographical error: In the ComposioToolSet init docstring, 'Heres' should be corrected to "Here's". - 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_t1LHt2Rj8I0VRPvi
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
@claude whats happenign in this PR |
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.
Fixes ENG-3668 Added python SDK support for create integration and initiate connection V2 logic of Hermes.
Tested using staging env
Create integration output on staging
Initiate connection output on staging
Important
Update Python SDK to support v2 logic for creating integrations and initiating connections, deprecating certain parameters and adding new tests.
initiate_connectionin__init__.pyto use v2 logic, deprecatingforce_new_integration.createincollections.pyto useapp_unique_keyinstead ofapp_id.initiateincollections.pyto use v2 endpointinitiateConnection.endpoint_v2forConnectedAccountsandIntegrationsincollections.py.endpoints.pyto include v2 endpoints forintegrations.force_new_integrationparameter intoolset.pyand__init__.py.TestIntegrationsConnectionsintest_toolset.pyto test v2 integration and connection logic.This description was created by
for e57e43e. It will automatically update as commits are pushed.