-
Notifications
You must be signed in to change notification settings - Fork 8.2k
⚡️ Speed up function get_cache_service by 1,090% in PR #11129 (aka-merge-1.7.1)
#11132
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
base: main
Are you sure you want to change the base?
Changes from all commits
2c3fe7f
168b84c
0833dfb
5040efc
c46d31c
2d735a7
17aff85
956baf6
4529e92
e89bee0
f7a82f3
73120ad
e321e3e
423419e
a562670
4df89f1
c66c679
2a3d424
731cc8b
3a2395b
9be8720
5b09d60
97164d8
c22ebff
7f5940e
d04142b
a1ce944
a9ef7fb
05d5a1e
efcae53
1174a6a
99e73b6
3beffea
f312f22
5ccd44e
a62b851
8245904
44a9f70
5b7e332
3c1d0d2
6566275
22c9237
5cffeae
6a0fd4d
2f157c9
5550861
5e54758
af529b3
b6ed2bc
f184989
7b66dfb
76af6b9
c160933
3200a2e
ac38023
7ba8c73
566a00a
e3782ee
4f4cb2f
966c223
0bfe12e
c067002
3a92285
16303cc
5160138
5227f61
79938ae
8b085de
9d44f3d
ec09b2b
0236d9a
072e94e
2140a7f
be0c024
d435761
cd90e87
b884fe4
197b5af
b26d032
b6a5fb0
8b0f727
783aba7
f4ed889
47ce5bf
6aa9178
1796285
998244d
b3b700f
1e6f648
8d9080a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,9 +10,11 @@ | |
| from langflow.api.utils import DbSession, custom_params | ||
| from langflow.schema.message import MessageResponse | ||
| from langflow.services.auth.utils import get_current_active_user | ||
| from langflow.services.database.models.flow.model import Flow | ||
| from langflow.services.database.models.message.model import MessageRead, MessageTable, MessageUpdate | ||
| from langflow.services.database.models.transactions.crud import transform_transaction_table | ||
| from langflow.services.database.models.transactions.model import TransactionTable | ||
| from langflow.services.database.models.user.model import User | ||
| from langflow.services.database.models.vertex_builds.crud import ( | ||
| delete_vertex_builds_by_flow_id, | ||
| get_vertex_builds_by_flow_id, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 HIGH - Missing authorization on vertex builds endpoint Description: Suggestion: Confidence: 90% |
||
|
|
@@ -39,15 +41,20 @@ async def delete_vertex_builds(flow_id: Annotated[UUID, Query()], session: DbSes | |
| raise HTTPException(status_code=500, detail=str(e)) from e | ||
|
|
||
|
|
||
| @router.get("/messages/sessions", dependencies=[Depends(get_current_active_user)]) | ||
| @router.get("/messages/sessions") | ||
| async def get_message_sessions( | ||
| session: DbSession, | ||
|
Comment on lines
41
to
46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 HIGH - Missing authorization on vertex builds deletion Description: Suggestion: Confidence: 90% |
||
| current_user: Annotated[User, Depends(get_current_active_user)], | ||
| flow_id: Annotated[UUID | None, Query()] = None, | ||
| ) -> list[str]: | ||
| try: | ||
| stmt = select(MessageTable.session_id).distinct() | ||
| stmt = stmt.where(col(MessageTable.session_id).isnot(None)) | ||
|
|
||
| # Filter by user's flows | ||
| user_flows_stmt = select(Flow.id).where(Flow.user_id == current_user.id) | ||
| stmt = stmt.where(col(MessageTable.flow_id).in_(user_flows_stmt)) | ||
|
|
||
| if flow_id: | ||
| stmt = stmt.where(MessageTable.flow_id == flow_id) | ||
|
|
||
|
|
@@ -57,9 +64,10 @@ async def get_message_sessions( | |
| raise HTTPException(status_code=500, detail=str(e)) from e | ||
|
|
||
|
|
||
| @router.get("/messages", dependencies=[Depends(get_current_active_user)]) | ||
| @router.get("/messages") | ||
| async def get_messages( | ||
| session: DbSession, | ||
| current_user: Annotated[User, Depends(get_current_active_user)], | ||
| flow_id: Annotated[UUID | None, Query()] = None, | ||
| session_id: Annotated[str | None, Query()] = None, | ||
| sender: Annotated[str | None, Query()] = None, | ||
|
|
@@ -68,6 +76,11 @@ async def get_messages( | |
| ) -> list[MessageResponse]: | ||
| try: | ||
| stmt = select(MessageTable) | ||
|
|
||
| # Filter by user's flows | ||
| user_flows_stmt = select(Flow.id).where(Flow.user_id == current_user.id) | ||
| stmt = stmt.where(col(MessageTable.flow_id).in_(user_flows_stmt)) | ||
|
|
||
| if flow_id: | ||
| stmt = stmt.where(MessageTable.flow_id == flow_id) | ||
| if session_id: | ||
|
|
@@ -80,8 +93,8 @@ async def get_messages( | |
| if sender_name: | ||
| stmt = stmt.where(MessageTable.sender_name == sender_name) | ||
| if order_by: | ||
| col = getattr(MessageTable, order_by).asc() | ||
| stmt = stmt.order_by(col) | ||
| order_col = getattr(MessageTable, order_by).asc() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM - Dynamic attribute access without validation Description: Suggestion: Confidence: 80%
Comment on lines
95
to
+96
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 HIGH - Unsafe dynamic attribute access allows arbitrary column access Description: Suggestion: Confidence: 90% There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 LOW - Dynamic column ordering needs validation Description: Suggestion: Confidence: 65% |
||
| stmt = stmt.order_by(order_col) | ||
| messages = await session.exec(stmt) | ||
| return [MessageResponse.model_validate(d, from_attributes=True) for d in messages] | ||
| except Exception as e: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 CRITICAL - Missing resource-level authorization on message deletion Description: Suggestion: Confidence: 95% There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 CRITICAL - Missing resource-level authorization on message update Description: Suggestion: Confidence: 95% |
||
|
|
||
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -164,7 +164,9 @@ async def aupdate_messages(messages: Message | list[Message]) -> list[Message]: | |
| # Convert flow_id to UUID if it's a string preventing error when saving to database | ||
| if msg.flow_id and isinstance(msg.flow_id, str): | ||
| msg.flow_id = UUID(msg.flow_id) | ||
| session.add(msg) | ||
| result = session.add(msg) | ||
| if asyncio.iscoroutine(result): | ||
| await result | ||
| updated_messages.append(msg) | ||
|
Comment on lines
+168
to
170
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM - Conditional await for session.add() Description: Suggestion: Confidence: 70% |
||
| else: | ||
| error_message = f"Message with id {message.id} not found" | ||
|
|
@@ -190,7 +192,9 @@ async def aadd_messagetables(messages: list[MessageTable], session: AsyncSession | |
| try: | ||
| try: | ||
| for message in messages: | ||
| session.add(message) | ||
| result = session.add(message) | ||
| if asyncio.iscoroutine(result): | ||
| await result | ||
| await session.commit() | ||
| # This is a hack. | ||
| # We are doing this because build_public_tmp causes the CancelledError to be raised | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 HIGH - CancelledError caught and converted to ValueError with retry Description: Suggestion: Confidence: 90% There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM - Second CancelledError handler converts to ValueError Description: Suggestion: Confidence: 85% |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from contextlib import asynccontextmanager | ||
| from typing import TYPE_CHECKING | ||
| from typing import TYPE_CHECKING, Union | ||
|
|
||
| from langflow.services.schema import ServiceType | ||
|
|
||
|
|
@@ -176,15 +176,18 @@ | |
| yield session | ||
|
|
||
|
|
||
| def get_cache_service() -> CacheService | AsyncBaseCacheService: | ||
| def get_cache_service() -> Union[CacheService, AsyncBaseCacheService]: # noqa: UP007 | ||
| """Retrieves the cache service from the service manager. | ||
| Returns: | ||
| The cache service instance. | ||
| """ | ||
| from langflow.services.cache.factory import CacheServiceFactory | ||
| if not hasattr(get_cache_service, "_factory"): | ||
| from langflow.services.cache.factory import CacheServiceFactory | ||
|
|
||
| return get_service(ServiceType.CACHE_SERVICE, CacheServiceFactory()) | ||
| get_cache_service._factory = CacheServiceFactory() | ||
|
|
||
| return get_service(ServiceType.CACHE_SERVICE, get_cache_service._factory) | ||
|
Comment on lines
+179
to
+190
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM - Function attribute used for caching Description: Suggestion: Confidence: 65%
Comment on lines
+185
to
+190
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 LOW - Function attribute used for caching without documentation Description: Suggestion: Confidence: 70% |
||
|
|
||
|
|
||
| def get_shared_component_cache_service() -> CacheService: | ||
|
|
||
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.
🟠 HIGH - Missing None check before dictionary access
Category: bug
Description:
In the remove_api_keys function, the code accesses node.get('data').get('node') without checking if node.get('data') returns None. This can cause an AttributeError at runtime.
Suggestion:
Add proper None checks:
node_data = node.get('data'); if node_data: node_data = node_data.get('node')Confidence: 95%
Rule:
bug_null_pointer_python