Closed utkarsh-dixit closed 1 month ago
Your free trial has expired. To keep using Ellipsis, sign up at https://app.ellipsis.dev for $20/seat/month or reach us at help@ellipsis.dev
β±οΈ Estimated effort to review: 2 π΅π΅βͺβͺβͺ |
π§ͺ No relevant tests |
π No security concerns identified |
β‘ Key issues to review Hardcoded Fallback URL The fallback URL 'https://backend.composio.dev/api' is hardcoded. Consider making it configurable or documenting why this specific URL is used as a fallback. Method Conversion Conversion of 'getApiUrlBase' from a private instance method to a public static method could lead to unintended usage or side effects. Review the access level and usage across the project. |
Category | Suggestion | Score |
Security |
β
Add a check for
___
| 9 |
Best practice |
Improve environment variable naming for clarity and compatibility___ **Consider using a more specific environment variable name or prefixing it withREACT_APP_ to ensure it is included during the build process in environments like Create React App. This change enhances clarity and compatibility.** [js/src/sdk/client/core/OpenAPI.ts [43]](https://github.com/ComposioHQ/composio/pull/337/files#diff-869b16570245c550781ad327f96ca31ba0739c58b47b0ee310082f1c852955f3R43-R43) ```diff -BASE: process.env.COMPOSIO_BASE_URL || 'https://backend.composio.dev/api', +BASE: process.env.REACT_APP_COMPOSIO_BASE_URL || 'https://backend.composio.dev/api', ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 8Why: This suggestion improves the clarity and compatibility of the environment variable, especially in environments like Create React App. It is a best practice to use a more specific name or prefix. | 8 |
Maintainability |
Refactor to reduce redundancy and improve maintainability___ **Refactor the repeated use ofbaseUrl || this.getApiUrlBase() by creating a helper function to get the effective base URL. This will improve code maintainability and reduce redundancy.** [js/src/sdk/index.ts [74]](https://github.com/ComposioHQ/composio/pull/337/files#diff-de82114af414fc4b45690c3f12d436b62d6270bd11e7b985bc37de196609651eR74-R74) ```diff -baseURL: baseUrl || this.getApiUrlBase(), +baseURL: getEffectiveBaseUrl(baseUrl), ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7Why: Creating a helper function to get the effective base URL reduces redundancy and improves maintainability. However, the actual implementation of the helper function is not provided, so the suggestion is partially incomplete. | 7 |
**Action:** test (ubuntu-latest, 3.10) |
**Failed stage:** [Unittests](https://github.com/ComposioHQ/composio/actions/runs/10020280749/job/27697748839) [β] |
**Failed test name:** test_action_enum |
**Failure summary:**
The action failed because the test test_action_enum in the file tests/test_client/test_enum.py raised a ValueError . GITHUB_ISSUES_LIST for Action ".GITHUB_ISSUES_LIST is not a valid value for the Action enum. |
Relevant error logs:```yaml 1: ##[group]Operating System 2: Ubuntu ... 506: * [new branch] fix/readme -> origin/fix/readme 507: * [new branch] fix/readme-logo -> origin/fix/readme-logo 508: * [new branch] fix/swe-agent -> origin/fix/swe-agent 509: * [new branch] ft-add-better-help-text -> origin/ft-add-better-help-text 510: * [new branch] ft-apps-id -> origin/ft-apps-id 511: * [new branch] ft-bring-back-core-sdk -> origin/ft-bring-back-core-sdk 512: * [new branch] ft-custom-base-url-js-sdk -> origin/ft-custom-base-url-js-sdk 513: * [new branch] ft-did-you-mean -> origin/ft-did-you-mean 514: * [new branch] ft-error-tracking -> origin/ft-error-tracking ... 930: tests/test_cli/test_actions.py::TestListActions::test_list_all[arguments3-exptected_outputs3-unexptected_outputs3] PASSED [ 17%] 931: tests/test_cli/test_actions.py::TestListActions::test_tag_not_found PASSED [ 19%] 932: tests/test_cli/test_actions.py::TestListActions::test_limit SKIPPED [ 21%] 933: tests/test_cli/test_actions.py::TestListActions::test_copy PASSED [ 23%] 934: tests/test_cli/test_add.py::TestComposioAdd::test_no_auth PASSED [ 25%] 935: tests/test_cli/test_apps.py::TestList::test_list PASSED [ 27%] 936: tests/test_cli/test_apps.py::TestUpdate::test_update_not_required PASSED [ 29%] 937: tests/test_cli/test_apps.py::TestUpdate::test_update SKIPPED (Needs 938: investigation, this test fails in CI) [ 31%] ... 946: tests/test_client/test_client.py::test_raise_invalid_api_key PASSED [ 48%] 947: tests/test_client/test_collections.py::TestTriggerNamesSerialization::test_converts_trigger_objects_to_comma_separated_string PASSED [ 51%] 948: tests/test_client/test_collections.py::TestTriggerNamesSerialization::test_converts_trigger_strings_to_comma_separated_string PASSED [ 53%] 949: tests/test_client/test_collections.py::TestTriggerNamesSerialization::test_converts_mix_of_trigger_objects_and_strings PASSED [ 55%] 950: tests/test_client/test_collections.py::test_trigger_subscription PASSED [ 57%] 951: tests/test_client/test_endpoints.py::test_endpoint PASSED [ 59%] 952: tests/test_client/test_enum.py::test_tag_enum PASSED [ 61%] 953: tests/test_client/test_enum.py::test_app_enum PASSED [ 63%] 954: tests/test_client/test_enum.py::test_action_enum FAILED [ 65%] ... 963: tests/test_tools/test_local/test_workspace.py::test_stderr PASSED [ 85%] 964: tests/test_tools/test_local/test_workspace.py::test_workspace PASSED [ 87%] 965: tests/test_utils/test_decorators.py::test_deprecated PASSED [ 89%] 966: tests/test_utils/test_git.py::test_get_git_user_info PASSED [ 91%] 967: tests/test_utils/test_shared.py::test_get_pydantic_signature_format_from_schema_params PASSED [ 93%] 968: tests/test_utils/test_shared.py::test_json_schema_to_pydantic_field PASSED [ 95%] 969: tests/test_utils/test_shared.py::test_json_schema_to_fields_dict PASSED [ 97%] 970: tests/test_utils/test_url.py::test_get_web_url PASSED [100%] 971: =================================== FAILURES =================================== ... 981: self = |
User description
β¦v variable
PR Type
Enhancement
Description
BASE
URL configurable via theCOMPOSIO_BASE_URL
environment variable inOpenAPIConfig
.Composio
class to use theCOMPOSIO_BASE_URL
environment variable forbaseUrl
initialization.getApiUrlBase
to a static method and updated relevant methods to use this new static method.0.1.5
to0.1.6-1
inpackage.json
.Changes walkthrough π
OpenAPI.ts
Make BASE URL configurable via environment variable
js/src/sdk/client/core/OpenAPI.ts
BASE
URL to be configurable viaCOMPOSIO_BASE_URL
environmentvariable.
index.ts
Update baseUrl initialization and methods to use environment variable
js/src/sdk/index.ts
baseUrl
initialization to useCOMPOSIO_BASE_URL
environmentvariable.
getApiUrlBase
to a static method and made it useCOMPOSIO_BASE_URL
.getApiUrlBase
static method.package.json
Bump package version to 0.1.6-1
js/package.json - Updated package version from `0.1.5` to `0.1.6-1`.