fix: actually populate logs#10398
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe Changes
Sequence DiagramsequenceDiagram
participant Caller
participant log_transaction
participant ImportAttempt as Import<br/>Langflow Models
participant DB as Database<br/>Session
participant Logger
Caller->>log_transaction: call with transaction data
log_transaction->>ImportAttempt: try import transaction models
alt ImportError
ImportAttempt-->>log_transaction: ImportError
log_transaction->>Logger: debug log (fallback)
else Import Success
ImportAttempt-->>log_transaction: models available
log_transaction->>log_transaction: construct TransactionBase
log_transaction->>DB: persist transaction
alt DB Success
DB-->>log_transaction: success
log_transaction->>Logger: log success message
else DB Error
DB-->>log_transaction: exception
log_transaction->>Logger: log error + debug log
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (4 passed)
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 |
github-actions
Bot
added
bug
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/lfx/src/lfx/graph/utils.py (2)
112-114: Tighten parameter types for clarity and safety.Annotate
statusanderrorexplicitly; keepsTransactionBase.statusconsistent and clarifies accepted error types.-async def log_transaction( - flow_id: str | UUID, - source: Vertex, - status, - target: Vertex | None = None, - error=None, -) -> None: +async def log_transaction( + flow_id: str | UUID, + source: Vertex, + status: str, + target: Vertex | None = None, + error: Exception | str | None = None, +) -> None:
121-129: Debug logging suggestion for disabled transaction storage is valid.The flag name
transactions_storage_enabledis correctly defined in the settings model and used throughout the codebase. The suggested debug log is a reasonable optional improvement that aligns with existing debug logging patterns in the same function (e.g., thedb_service is Nonecheck on line 128). The developer can implement this at their discretion.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lfx/src/lfx/graph/utils.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lfx/src/lfx/graph/utils.py (2)
src/backend/base/langflow/services/database/models/transactions/model.py (1)
TransactionBase(10-55)src/backend/base/langflow/services/deps.py (1)
session_scope(162-184)
🪛 GitHub Actions: Ruff Style Check
src/lfx/src/lfx/graph/utils.py
[error] 138-138: Ruff: Import block is unsorted or unformatted (I001). Command 'uv run --only-dev ruff check --output-format=github .' reported the issue.
🪛 GitHub Check: Ruff Style Check (3.13)
src/lfx/src/lfx/graph/utils.py
[failure] 166-166: Ruff (W293)
src/lfx/src/lfx/graph/utils.py:166:1: W293 Blank line contains whitespace
[failure] 162-162: Ruff (BLE001)
src/lfx/src/lfx/graph/utils.py:162:16: BLE001 Do not catch blind exception: Exception
[failure] 158-158: Ruff (W293)
src/lfx/src/lfx/graph/utils.py:158:1: W293 Blank line contains whitespace
[failure] 156-156: Ruff (W293)
src/lfx/src/lfx/graph/utils.py:156:1: W293 Blank line contains whitespace
[failure] 152-152: Ruff (W293)
src/lfx/src/lfx/graph/utils.py:152:1: W293 Blank line contains whitespace
[failure] 147-147: Ruff (Q000)
src/lfx/src/lfx/graph/utils.py:147:89: Q000 Single quotes found but double quotes preferred
[failure] 146-146: Ruff (Q000)
src/lfx/src/lfx/graph/utils.py:146:77: Q000 Single quotes found but double quotes preferred
[failure] 141-141: Ruff (W293)
src/lfx/src/lfx/graph/utils.py:141:1: W293 Blank line contains whitespace
[failure] 138-140: Ruff (I001)
src/lfx/src/lfx/graph/utils.py:138:13: I001 Import block is un-sorted or un-formatted
⏰ 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: Update Component Index
- GitHub Check: Update Starter Projects
🔇 Additional comments (2)
src/lfx/src/lfx/graph/utils.py (2)
117-119: Docstring update reads well.Accurately reflects the new DB‑backed logging with graceful fallback.
154-156: DB logging path LGTM.Async session usage + single write via
db_log_transactionis appropriate.
| # Try to use the full langflow transaction logging implementation | ||
| try: | ||
| from langflow.services.database.models.transactions.model import TransactionBase | ||
| from langflow.services.database.models.transactions.crud import log_transaction as db_log_transaction | ||
| from langflow.services.deps import session_scope | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix import order to satisfy Ruff (I001).
Sort the inner import block alphabetically by module path.
- from langflow.services.database.models.transactions.model import TransactionBase
- from langflow.services.database.models.transactions.crud import log_transaction as db_log_transaction
- from langflow.services.deps import session_scope
+ from langflow.services.database.models.transactions.crud import log_transaction as db_log_transaction
+ from langflow.services.database.models.transactions.model import TransactionBase
+ from langflow.services.deps import session_scope🧰 Tools
🪛 GitHub Actions: Ruff Style Check
[error] 138-138: Ruff: Import block is unsorted or unformatted (I001). Command 'uv run --only-dev ruff check --output-format=github .' reported the issue.
🪛 GitHub Check: Ruff Style Check (3.13)
[failure] 141-141: Ruff (W293)
src/lfx/src/lfx/graph/utils.py:141:1: W293 Blank line contains whitespace
[failure] 138-140: Ruff (I001)
src/lfx/src/lfx/graph/utils.py:138:13: I001 Import block is un-sorted or un-formatted
🤖 Prompt for AI Agents
In src/lfx/src/lfx/graph/utils.py around lines 136 to 141, the inner try-import
block is not alphabetically ordered causing Ruff I001; reorder the three imports
alphabetically by their full module path so the import lines read in ascending
lexical order (i.e., ensure the module paths are sorted) and preserve them
inside the try block exactly as before.
| from langflow.services.database.models.transactions.model import TransactionBase | ||
| from langflow.services.database.models.transactions.crud import log_transaction as db_log_transaction | ||
| from langflow.services.deps import session_scope | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Strip trailing whitespace on blank lines (W293).
Ruff flags these blanks as containing whitespace. Remove trailing spaces to pass style checks.
Also applies to: 152-152, 156-156, 158-158, 166-166
🧰 Tools
🪛 GitHub Check: Ruff Style Check (3.13)
[failure] 141-141: Ruff (W293)
src/lfx/src/lfx/graph/utils.py:141:1: W293 Blank line contains whitespace
🤖 Prompt for AI Agents
In src/lfx/src/lfx/graph/utils.py around lines 141, 152, 156, 158, and 166,
several blank lines contain trailing whitespace (Ruff W293); remove any trailing
spaces on those blank lines so they are truly empty (no spaces or tabs), re-save
the file, and re-run the linter to confirm W293 is resolved.
| transaction_data = TransactionBase( | ||
| vertex_id=source.id, | ||
| target_id=target.id if target else None, | ||
| inputs=_vertex_to_primitive_dict(source) if hasattr(source, 'data') else None, | ||
| outputs=_vertex_to_primitive_dict(target) if target and hasattr(target, 'data') else None, | ||
| status=status, | ||
| error=str(error) if error else None, | ||
| flow_id=flow_id, | ||
| ) |
There was a problem hiding this comment.
Bug: inputs/outputs guards check .data but serializer reads .params.
_vertex_to_primitive_dict accesses vertex.params. Using hasattr(..., "data") will often return False, causing inputs/outputs to be None and logs to remain empty.
- transaction_data = TransactionBase(
- vertex_id=source.id,
- target_id=target.id if target else None,
- inputs=_vertex_to_primitive_dict(source) if hasattr(source, 'data') else None,
- outputs=_vertex_to_primitive_dict(target) if target and hasattr(target, 'data') else None,
- status=status,
- error=str(error) if error else None,
- flow_id=flow_id,
- )
+ transaction_data = TransactionBase(
+ vertex_id=source.id,
+ target_id=target.id if target else None,
+ inputs=_vertex_to_primitive_dict(source),
+ outputs=_vertex_to_primitive_dict(target) if target else None,
+ status=str(status),
+ error=str(error) if error is not None else None,
+ flow_id=flow_id,
+ )Also addresses Q000 by using double quotes in the modified lines.
📝 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.
| transaction_data = TransactionBase( | |
| vertex_id=source.id, | |
| target_id=target.id if target else None, | |
| inputs=_vertex_to_primitive_dict(source) if hasattr(source, 'data') else None, | |
| outputs=_vertex_to_primitive_dict(target) if target and hasattr(target, 'data') else None, | |
| status=status, | |
| error=str(error) if error else None, | |
| flow_id=flow_id, | |
| ) | |
| transaction_data = TransactionBase( | |
| vertex_id=source.id, | |
| target_id=target.id if target else None, | |
| inputs=_vertex_to_primitive_dict(source), | |
| outputs=_vertex_to_primitive_dict(target) if target else None, | |
| status=str(status), | |
| error=str(error) if error is not None else None, | |
| flow_id=flow_id, | |
| ) |
🧰 Tools
🪛 GitHub Check: Ruff Style Check (3.13)
[failure] 147-147: Ruff (Q000)
src/lfx/src/lfx/graph/utils.py:147:89: Q000 Single quotes found but double quotes preferred
[failure] 146-146: Ruff (Q000)
src/lfx/src/lfx/graph/utils.py:146:77: Q000 Single quotes found but double quotes preferred
🤖 Prompt for AI Agents
In src/lfx/src/lfx/graph/utils.py around lines 143 to 151, the guard checks use
hasattr(vertex, 'data') but _vertex_to_primitive_dict reads vertex.params,
causing inputs/outputs to be None; update the conditional guards to check for
"params" (e.g., hasattr(source, "params") and hasattr(target, "params")) so the
serializer is invoked when params exist, and use double-quoted strings in the
modified lines.
| # Fallback to debug logging if langflow models are not available | ||
| logger.debug(f"Transaction logged (debug only): vertex={source.id}, flow={flow_id}, status={status}") | ||
| except Exception as db_exc: | ||
| # Log database errors but don't fail the transaction | ||
| logger.debug(f"Error logging transaction to database: {db_exc!s}") | ||
| logger.debug(f"Transaction logged (debug only): vertex={source.id}, flow={flow_id}, status={status}") | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Ruff BLE001 and better diagnostics for DB logging failures.
Either narrow the exception types or mirror the outer pattern and suppress BLE001 here. Minimal change below also keeps CI green.
- except ImportError:
+ except ImportError:
# Fallback to debug logging if langflow models are not available
logger.debug(f"Transaction logged (debug only): vertex={source.id}, flow={flow_id}, status={status}")
- except Exception as db_exc:
+ except Exception as db_exc: # noqa: BLE001
# Log database errors but don't fail the transaction
- logger.debug(f"Error logging transaction to database: {db_exc!s}")
+ logger.debug(f"Error logging transaction to database: {db_exc!s}")
logger.debug(f"Transaction logged (debug only): vertex={source.id}, flow={flow_id}, status={status}")Optional (recommended later): catch (ValueError, sqlalchemy.exc.SQLAlchemyError, pydantic.ValidationError) explicitly if those are guaranteed to be present.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: Ruff Style Check (3.13)
[failure] 166-166: Ruff (W293)
src/lfx/src/lfx/graph/utils.py:166:1: W293 Blank line contains whitespace
[failure] 162-162: Ruff (BLE001)
src/lfx/src/lfx/graph/utils.py:162:16: BLE001 Do not catch blind exception: Exception
🤖 Prompt for AI Agents
In src/lfx/src/lfx/graph/utils.py around lines 160 to 166, change the bare
"except Exception" used for DB logging to a narrower explicit exception tuple
(e.g., except (ValueError, sqlalchemy.exc.SQLAlchemyError,
pydantic.ValidationError):) and add the necessary imports for sqlalchemy.exc and
pydantic.ValidationError at the top of the file; if any of those packages are
not available in this module's scope, alternatively keep Exception but append a
noqa comment to suppress BLE001 (e.g., # noqa: BLE001) and ensure the debug
messages remain the same so CI stays green.
|
Thanks, @ogabrielluiz! Does #10136 fix this issue to still show logs? |
2 participants
Not sure why, but logs are never populated. After looking into it, I found out it's because the
log_transactionfunction in the LFX codebase was only a stub implementation that logged to the debug logger but never actually saved transactions to the database. This PR fixes this.Current State
Screen.Recording.2025-10-24.at.17.53.19.mov
This PR
Screen.Recording.2025-10-24.at.17.54.28.mov
Summary by CodeRabbit