fix: duplicate account connection on basic auth

[tushar-composio]

initiate_connection() now creates the connected account with the auth config properly. No need to create another one.

---

Important

Fixes duplicate account connection issue in basic auth by removing redundant function call in add.py and refactoring related code.

• Behavior:
  • Removes redundant save_user_access_data() call in _handle_basic_auth() in add.py, preventing duplicate account connections.
• Functions:
  • Adds _get_connected_account() in runtime.py to retrieve connected account details.
  • Modifies _get_auth_params() in runtime.py to use ConnectedAccountModel.
• Misc:
  • Minor refactor in _handle_basic_auth() in add.py to streamline connection process.
  • Updates _build_executable_from_args() in runtime.py to handle connected_account parameter.

^{This description was created by for e821402. It will automatically update as commits are pushed.}

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
composio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 23, 2024 0:48am

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 12ae898 in 7 seconds

More details

• Looked at 16 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 drafted comments based on config settings.

1. python/composio/cli/add.py:336

• Draft comment:
  Ensure that removing save_user_access_data does not affect other parts of the code that might rely on it. The change seems correct as per the PR description, but double-check for any dependencies.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The removal of the save_user_access_data call seems appropriate given the PR description, which states that initiate_connection() now handles the creation of the connected account with the auth config properly. However, it's important to ensure that this change doesn't affect any other part of the code that might rely on save_user_access_data.

Workflow ID: wflow_N0y6TQHCMAEN0o4q

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[shreysingla11]

Consider adding a comment here explaining that user access data is already saved during connection initialization to prevent future reintroduction of duplicate calls.

[shreysingla11]

Code Review Summary

The changes look good and address the issue of duplicate account connection handling during basic authentication. Here's my assessment:

Positive Points:

• ✅ Removes redundant save_user_access_data call that was causing duplicate operations
• ✅ Change is minimal and focused on fixing a specific issue
• ✅ No functionality is broken as user access data is already saved during connection initialization

Suggestions:

• Consider adding a comment explaining why this code was removed to prevent future reintroduction
• The _handle_basic_auth function could benefit from more detailed docstring explaining the authentication flow

Overall code quality: 8/10 - Clean, focused fix that improves code efficiency by removing redundant operations.

[ellipsis-dev]

👍 Looks good to me! Incremental review on e821402 in 1 minute and 7 seconds

More details

• Looked at 75 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 3 drafted comments based on config settings.

1. python/composio/tools/base/runtime.py:314

• Draft comment:
  The variable auth_params is renamed to auth_param in the PR, but the change is not applied consistently throughout the file. Ensure consistent naming to avoid confusion.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment appears to be incorrect. The variable auth_params only appears once in the original code at line 317, and it was renamed to auth_param. There are no other instances of this variable that were missed. The comment seems to have confused auth_params with connection_params, which is a different variable used for a different purpose.
  Could there be other files where this variable is used that I'm not seeing? Could connection_params actually be related to auth_params in some way I'm missing?
  No - the rule is to ignore cross-file issues, and connection_params is clearly a different variable used to store the actual connection parameters from the API response, while auth_param is a boolean flag used to track whether auth is needed.
  The comment is incorrect - there is no inconsistency in the renaming as the variable only appears once. The comment should be deleted.

2. python/composio/tools/base/runtime.py:328

• Draft comment:
  The variable auth_params is renamed to auth_param in the PR, but the change is not applied consistently throughout the file. Ensure consistent naming to avoid confusion.
• Reason this comment was not posted:
  Marked as duplicate.

3. python/composio/tools/base/runtime.py:392

• Draft comment:
  The variable auth_params is renamed to auth_param in the PR, but the change is not applied consistently throughout the file. Ensure consistent naming to avoid confusion.
• Reason this comment was not posted:
  Marked as duplicate.

Workflow ID: wflow_5QL4ps4ukSBNOxyB

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.