feat: pass context argument to tasks that require it #788
Conversation
Please make sure all the checkboxes are checked:
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes introduce an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant run_tasks
participant run_tasks_with_telemetry
participant run_tasks_base
participant handle_task
participant Task
Caller->>run_tasks: run_tasks(tasks, ..., context)
run_tasks->>run_tasks_with_telemetry: run_tasks_with_telemetry(..., context)
run_tasks_with_telemetry->>run_tasks_base: run_tasks_base(..., context)
loop For each task
run_tasks_base->>handle_task: handle_task(running_task, ..., context)
alt Task accepts context
handle_task->>Task: Task(..., context)
else Task does not accept context
handle_task->>Task: Task(...)
end
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cognee/shared/logging_utils.py (1)
315-315: Broadened SQLAlchemy warning suppression scopeThis change modifies when SQLAlchemy warnings are suppressed, now filtering them at all log levels above DEBUG (previously only above WARNING). While this reduces log noise at INFO and WARNING levels, consider whether this aligns with your overall logging strategy.
Since this is a behavioral change, consider adding a brief comment explaining the rationale for filtering SQLAlchemy warnings at these levels.
+ # Filter SQLAlchemy warnings at all levels above DEBUG to reduce noise while preserving debug-level visibility if log_level > logging.DEBUG:Verify that this change is intentional and that important SQLAlchemy warnings that should be visible at INFO or WARNING levels aren't being suppressed. This may be related to the new context parameter implementation in tasks.
cognee/modules/pipelines/operations/run_tasks_base.py (1)
31-37: Good implementation of context detection and injectionThis is a clean implementation that:
- Inspects the task's function signature to check if it accepts a "context" parameter
- Only provides the context to tasks that expect it
Minor optimization suggested:
- has_context = any( - [key == "context" for key in inspect.signature(running_task.executable).parameters.keys()] - ) + has_context = "context" in inspect.signature(running_task.executable).parametersThis change avoids creating a temporary list and simplifies the check, while also addressing the SIM118 static analysis hint.
🧰 Tools
🪛 Ruff (0.8.2)
32-32: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cognee/modules/pipelines/operations/run_tasks.py(4 hunks)cognee/modules/pipelines/operations/run_tasks_base.py(4 hunks)cognee/shared/logging_utils.py(1 hunks)cognee/tests/integration/run_toy_tasks/conftest.py(0 hunks)cognee/tests/unit/modules/pipelines/run_tasks_test.py(1 hunks)cognee/tests/unit/modules/pipelines/run_tasks_with_context_test.py(1 hunks)
💤 Files with no reviewable changes (1)
- cognee/tests/integration/run_toy_tasks/conftest.py
🧰 Additional context used
🧬 Code Graph Analysis (3)
cognee/tests/unit/modules/pipelines/run_tasks_test.py (1)
cognee/tests/unit/modules/pipelines/run_tasks_with_context_test.py (1)
test_run_tasks(42-43)
cognee/modules/pipelines/operations/run_tasks.py (2)
cognee/modules/pipelines/tasks/task.py (1)
Task(5-97)cognee/modules/pipelines/operations/run_tasks_base.py (1)
run_tasks_base(66-82)
cognee/tests/unit/modules/pipelines/run_tasks_with_context_test.py (4)
cognee/modules/pipelines/tasks/task.py (1)
Task(5-97)cognee/modules/users/methods/get_default_user.py (1)
get_default_user(12-37)cognee/modules/pipelines/operations/run_tasks_base.py (1)
run_tasks_base(66-82)cognee/tests/unit/modules/pipelines/run_tasks_test.py (1)
run_and_check_tasks(10-46)
🪛 Ruff (0.8.2)
cognee/modules/pipelines/operations/run_tasks_base.py
32-32: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Basic Tests / Run Unit Tests
- GitHub Check: Basic Tests / Run Simple Examples
- GitHub Check: Basic Tests / Run Basic Graph Tests
- GitHub Check: End-to-End Tests / S3 Bucket Test
- GitHub Check: End-to-End Tests / Deletion Test
- GitHub Check: End-to-End Tests / Server Start Test
- GitHub Check: End-to-End Tests / Run Telemetry Pipeline Test
- GitHub Check: End-to-End Tests / Deduplication Test
🔇 Additional comments (12)
cognee/tests/unit/modules/pipelines/run_tasks_test.py (1)
53-54: Good addition of an entry point!Adding the
if __name__ == "__main__":block allows this test module to be executed as a standalone script, which is a good Python practice. This aligns with the approach used in the new context-aware test module.cognee/tests/unit/modules/pipelines/run_tasks_with_context_test.py (3)
14-21: Well-structured task definitions with context supportGood job defining tasks with different signatures - some accepting context (task_1, task_3) and some without (task_2). This effectively tests how the pipeline properly provides context only to functions that need it.
26-35: Good test pipeline setup with context parameterThe pipeline is properly set up using
run_tasks_basewith the new context parameter. This tests the core functionality of passing context through the execution pipeline.
37-39: Correct verification of pipeline resultsThe test correctly verifies that the final result matches what's expected from the chain of operations:
- task_1(5, 7) = 12
- task_2(12) = 24
- task_3(24, 7) = 24^7 = 4,586,471,424
cognee/modules/pipelines/operations/run_tasks.py (4)
23-25: Good addition of context parameterThe
contextparameter is properly added to the function signature with a sensible default value (None). This maintains backward compatibility while supporting the new feature.
41-41: Proper context propagationThe context parameter is correctly forwarded to the
run_tasks_basefunction, ensuring it's available throughout the task execution pipeline.
77-77: Good addition of context parameterThe
contextparameter is properly added to therun_tasksfunction with a sensible default value (None), maintaining backward compatibility.
87-93: Good use of explicit parameter namesThe function call is now using explicit parameter names, which improves readability and makes it clear that context is being passed through the pipeline.
cognee/modules/pipelines/operations/run_tasks_base.py (4)
17-17: Good addition of context parameterThe
contextparameter is properly added to thehandle_taskfunction with a sensible default value (None), maintaining backward compatibility.
40-40: Proper recursive propagation of contextThe context parameter is correctly passed to the recursive call to
run_tasks_base, ensuring it's available for downstream tasks.
66-66: Good addition of context parameterThe
contextparameter is properly added to therun_tasks_basefunction with a sensible default value (None), maintaining backward compatibility.
79-81: Proper context forwardingThe context parameter is correctly passed to the
handle_taskfunction, ensuring tasks that need context can access it.
<!-- .github/pull_request_template.md --> ## Description <!-- Provide a clear description of the changes in this PR --> ## DCO Affirmation I affirm that all code in every commit of this pull request conforms to the terms of the Topoteretes Developer Certificate of Origin. --------- Co-authored-by: Vasilije <8619304+Vasilije1990@users.noreply.github.com>
…sks-that-require-it
…ks-that-require-it
…ks-that-require-it
Description
DCO Affirmation
I affirm that all code in every commit of this pull request conforms to the terms of the Topoteretes Developer Certificate of Origin.