add view for composio keys (integration authorizations) #836
Conversation
📝 WalkthroughWalkthroughAdds a new exported icon constant Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🧰 Additional context used🧬 Code graph analysis (1)functions/views.py (5)
🪛 Ruff (0.14.5)functions/views.py20-20: Probable use of (S113) 90-94: Parenthesize Parenthesize the (RUF021) 98-99: Parenthesize Parenthesize the (RUF021) 107-107: Parenthesize Parenthesize the (RUF021) 110-110: Parenthesize Parenthesize the (RUF021) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
functions/views.py (1)
79-91: Consider parenthesizingandexpressions for clarity.The chained
and/orexpressions could benefit from explicit parentheses to make operator precedence clear.Apply this diff to improve readability:
published_run = ( - published_run_id - and PublishedRun.objects.filter( + (published_run_id + and PublishedRun.objects.filter( published_run_id=published_run_id - ).first() + ).first()) or None ) app_user_id = scope_parts.get(ScopeParts.member) app_user = ( - app_user_id - and AppUser.objects.filter(id=app_user_id).first() + (app_user_id + and AppUser.objects.filter(id=app_user_id).first()) or None )functions/models.py (1)
159-174: Add explicitstrict=Trueto zip() for safety.The
zip()call should specifystrict=Trueto catch any unexpected mismatches between scope part keys and values.Apply this diff:
scope_values = { ScopeParts(part): value - for part, value in zip(user_id_scope_parts, parts[1::2]) + for part, value in zip(user_id_scope_parts, parts[1::2], strict=True) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
daras_ai_v2/icons.py(1 hunks)functions/composio_tools.py(1 hunks)functions/models.py(2 hunks)functions/recipe_functions.py(1 hunks)functions/views.py(1 hunks)routers/account.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
functions/models.py (2)
app_users/models.py (2)
AppUser(90-262)full_name(161-177)workspaces/models.py (2)
html_icon(409-416)Workspace(105-454)
functions/views.py (4)
functions/composio_tools.py (1)
get_toolkit_name_by_slug(270-275)functions/models.py (5)
FunctionScopes(40-214)ScopeParts(25-31)get_user_id_for_scope(181-214)from_user_id(160-174)format_label(86-157)app_users/models.py (1)
AppUser(90-262)bots/models/published_run.py (1)
PublishedRun(118-346)
routers/account.py (2)
functions/views.py (1)
manage_integration_authorizations(54-114)routers/root.py (1)
TabData(884-887)
🪛 Ruff (0.14.4)
functions/models.py
167-167: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
functions/views.py
20-20: Probable use of requests call without timeout
(S113)
80-83: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
88-89: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (11)
daras_ai_v2/icons.py (1)
6-6: LGTM! Clean addition of new icon constant.The
keysicon follows the established pattern and is appropriately used in the account settings UI.functions/views.py (2)
1-17: LGTM! Type hints improve code clarity.The addition of type hints and the TYPE_CHECKING block for forward references follows best practices for Python typing.
54-114: LGTM! Well-structured integration authorizations view.The function correctly:
- Fetches authorizations from Composio
- Filters by workspace scope
- Resolves toolkit names and related entities
- Renders a clear table with workflow, integration, and scope information
- Passes
current_userfor context-aware labelingrouters/account.py (3)
22-22: LGTM! Proper import of new view function.
256-258: LGTM! Title update reflects expanded functionality.The tab title now appropriately covers both API keys and integration authorizations, using the new
icons.keysconstant.
378-379: LGTM! Clean integration of the new authorizations UI.The new section is properly placed and passes the required workspace and user context.
functions/recipe_functions.py (1)
340-346: LGTM! Enables context-aware labeling.Adding
current_user=userto theformat_labelcall allows the scope selector to display personalized labels like "(You)" when appropriate.functions/composio_tools.py (1)
269-275: LGTM! Clean helper function with sensible fallback.The function provides a straightforward lookup mechanism and gracefully handles missing toolkits by returning the slug itself.
functions/models.py (3)
86-104: LGTM! Context-aware labeling improves UX.The addition of the
current_userparameter and the updated workspace label logic provide clearer, more personalized scope labels. The distinction between personal and team workspace labeling is well-implemented.
106-114: LGTM! Clear "(You)" indicator for current user.The logic correctly displays "Only I (FullName)" when the user matches the current user, improving the user experience by making it clear which scopes belong to them.
176-178: LGTM! Useful helper for workspace user_id prefix.This classmethod provides a clean way to generate the workspace prefix used for filtering authorizations.
| r = requests.get( | ||
| "https://backend.composio.dev/api/v3/connected_accounts", | ||
| headers={ | ||
| "x-api-key": str(settings.COMPOSIO_API_KEY), | ||
| "Content-Type": "application/json", | ||
| "Accept": "application/json", | ||
| }, | ||
| json={ | ||
| "statuses": ["ACTIVE"], | ||
| "limit": 10_000, | ||
| }, | ||
| ) | ||
| r.raise_for_status() |
There was a problem hiding this comment.
Add timeout to requests.get call.
The HTTP request lacks a timeout parameter, which could cause the application to hang indefinitely if the remote server is unresponsive.
Apply this diff to add a timeout:
r = requests.get(
"https://backend.composio.dev/api/v3/connected_accounts",
headers={
"x-api-key": str(settings.COMPOSIO_API_KEY),
"Content-Type": "application/json",
"Accept": "application/json",
},
json={
"statuses": ["ACTIVE"],
"limit": 10_000,
},
+ timeout=30,
)🧰 Tools
🪛 Ruff (0.14.4)
20-20: Probable use of requests call without timeout
(S113)
🤖 Prompt for AI Agents
In functions/views.py around lines 20 to 32, the requests.get call is missing a
timeout which can cause the app to hang indefinitely; update the call to include
a timeout parameter (for example timeout=10 or a tuple like timeout=(5, 10) for
connect/read) so the request fails fast on unresponsive servers and propagates
exceptions as before; keep the rest of the headers/json unchanged and ensure any
existing error handling (r.raise_for_status()) continues to work.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
functions/views.py (1)
20-31: Add timeout to the HTTP request.This issue was already flagged in the previous review.
🧹 Nitpick comments (1)
functions/views.py (1)
82-94: Consider adding parentheses for clarity.The
and/oroperator chaining works correctly but can be harder to read. Adding parentheses makes the precedence explicit.Apply this diff to improve readability:
published_runs = ( - pr_ids - and { + (pr_ids and { pr.published_run_id: pr for pr in PublishedRun.objects.filter(published_run_id__in=pr_ids) - } + }) or {} ) users = ( - user_ids - and {user.id: user for user in AppUser.objects.filter(id__in=user_ids)} + (user_ids and {user.id: user for user in AppUser.objects.filter(id__in=user_ids)}) or {} )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
functions/views.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
functions/views.py (5)
functions/composio_tools.py (1)
get_toolkit_name_by_slug(270-275)functions/models.py (5)
FunctionScopes(40-214)ScopeParts(25-31)get_user_id_for_scope(181-214)from_user_id(160-174)format_label(86-157)app_users/models.py (1)
AppUser(90-262)bots/models/published_run.py (1)
PublishedRun(118-346)workspaces/models.py (2)
Workspace(105-454)html_icon(409-416)
🪛 Ruff (0.14.4)
functions/views.py
20-20: Probable use of requests call without timeout
(S113)
83-87: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
91-92: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
100-100: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
103-103: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (python)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (2)
functions/views.py (2)
1-16: LGTM!The imports are well-organized, and the
TYPE_CHECKINGblock correctly handles the forward reference toWorkspaceto avoid circular imports.
54-128: Well-structured integration management UI.The function efficiently bulk-fetches related objects (published runs and users) to avoid N+1 queries, and the table rendering logic correctly handles various scope types and edge cases.
| workspace_user_id = FunctionScopes.get_user_id_for_scope( | ||
| scope=FunctionScopes.workspace, | ||
| workspace=workspace, | ||
| user=None, | ||
| published_run=None, | ||
| ) |
There was a problem hiding this comment.
Type mismatch: passing None to non-Optional parameters.
Looking at the signature of get_user_id_for_scope in functions/models.py (lines 180-213), the user and published_run parameters are typed as AppUser and PublishedRun without Optional. Passing None violates the type contract, even though it works at runtime for FunctionScopes.workspace scope.
Consider one of these solutions:
Solution 1: Cast to satisfy type checkers:
workspace_user_id = FunctionScopes.get_user_id_for_scope(
scope=FunctionScopes.workspace,
workspace=workspace,
- user=None,
- published_run=None,
+ user=typing.cast(AppUser, None),
+ published_run=typing.cast(PublishedRun, None),
)Solution 2: Update get_user_id_for_scope signature in functions/models.py to accept Optional types (if you have access to modify it).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| workspace_user_id = FunctionScopes.get_user_id_for_scope( | |
| scope=FunctionScopes.workspace, | |
| workspace=workspace, | |
| user=None, | |
| published_run=None, | |
| ) | |
| workspace_user_id = FunctionScopes.get_user_id_for_scope( | |
| scope=FunctionScopes.workspace, | |
| workspace=workspace, | |
| user=typing.cast(AppUser, None), | |
| published_run=typing.cast(PublishedRun, None), | |
| ) |
| published_run_id and published_runs.get(published_run_id) or None | ||
| ) | ||
| user_id = scope_parts.get(ScopeParts.member) | ||
| user = user_id and users.get(int(user_id)) or None |
There was a problem hiding this comment.
Add error handling for int() conversion.
The int(user_id) conversion could raise a ValueError if user_id contains non-numeric data. While unlikely given the source, defensive programming suggests handling this gracefully.
Apply this diff to handle the conversion safely:
-user = user_id and users.get(int(user_id)) or None
+try:
+ user = (user_id and users.get(int(user_id))) or None
+except (ValueError, TypeError):
+ user = NoneOr use a helper:
def safe_int(value):
try:
return int(value)
except (ValueError, TypeError):
return None
# Then:
user = (user_id and users.get(safe_int(user_id))) or None🧰 Tools
🪛 Ruff (0.14.4)
103-103: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
🤖 Prompt for AI Agents
In functions/views.py around line 103, the direct int(user_id) conversion can
raise ValueError/TypeError; replace it with a safe conversion and lookup (e.g.,
call a helper safe_int that returns None on invalid input) and use users.get on
the safe result, ensuring user becomes None when conversion fails; update
imports if you add the helper and write tests or checks to cover non-numeric
user_id inputs.
Q/A checklist
How to check import time?
You can visualize this using tuna:
To measure import time for a specific library:
To reduce import times, import libraries that take a long time inside the functions that use them instead of at the top of the file:
Legal Boilerplate
Look, I get it. The entity doing business as “Gooey.AI” and/or “Dara.network” was incorporated in the State of Delaware in 2020 as Dara Network Inc. and is gonna need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Dara Network Inc can use, modify, copy, and redistribute my contributions, under its choice of terms.