Feature/async support on pi run #1161
Conversation
…or sync mode w/ burnettk
… avoid confusing methods w/ burnettk
WalkthroughWalkthroughThe recent updates enhance the system by introducing an Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 8
Configuration used: .coderabbit.yaml
Files selected for processing (7)
- spiffworkflow-backend/src/spiffworkflow_backend/api.yml (2 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/background_processing/celery_tasks/process_instance_task_producer.py (2 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/helpers/spiff_enum.py (1 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py (4 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py (3 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py (1 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py (4 hunks)
Additional comments: 7
spiffworkflow-backend/src/spiffworkflow_backend/helpers/spiff_enum.py (1)
- 10-12: The addition of the
ProcessInstanceExecutionModeenum class is correctly implemented and follows Python's enum conventions. This will facilitate clear differentiation between execution modes in the system.spiffworkflow-backend/src/spiffworkflow_backend/background_processing/celery_tasks/process_instance_task_producer.py (1)
- 40-52: The modifications to handle different
execution_modevalues inqueue_process_instance_if_appropriateare well-implemented, ensuring that asynchronous execution is only attempted when Celery is enabled. This is crucial for maintaining system integrity and providing clear error messages to users.Consider adding a brief comment above the check for
execution_mode == ProcessInstanceExecutionMode.asynchronous.valueexplaining why this check is necessary, for future maintainability.spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py (2)
- 79-79: The introduction of the
execution_modeparameter in theprocess_instance_runfunction aligns with the PR's objective to support asynchronous execution. Ensure that the handling of this parameter is thoroughly tested, especially in scenarios where the execution mode impacts the process instance's behavior.- 658-672: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [650-669]
The handling of the
execution_modeparameter in the_process_instance_runfunction is well-implemented, with logic to adjust the execution strategy based on the specified mode. Consider adding explicit error handling for invalidexecution_modevalues to improve robustness. Ensure comprehensive testing, especially for edge cases and error scenarios.spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py (3)
- 563-563: The delegation of task submission logic to
_task_submit_sharedwith the inclusion of theexecution_modeparameter is a good practice, promoting code reuse and maintainability. This change aligns with the PR's objective to introduce asynchronous execution support.- 879-879: The
_task_submit_sharedfunction now accepts theexecution_modeparameter, which is crucial for handling the task submission based on the execution mode specified. It's important to ensure that the logic within this function correctly handles both synchronous and asynchronous modes as intended.Verify that the downstream logic within
_task_submit_shared, especially in theProcessInstanceService.complete_form_taskcall, properly utilizes theexecution_modeparameter to differentiate between synchronous and asynchronous task execution.
- 930-930: The
execution_modeparameter is passed to theProcessInstanceService.complete_form_taskmethod. This is a critical integration point for the asynchronous execution feature. It's essential to ensure that thecomplete_form_taskmethod is updated to handle this parameter appropriately, managing task completion based on the specified execution mode.Confirm that the
ProcessInstanceService.complete_form_taskmethod has been updated to support theexecution_modeparameter, effectively handling both synchronous and asynchronous execution modes.
spiffworkflow-backend/src/spiffworkflow_backend/routes/tasks_controller.py
Show resolved
Hide resolved
spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_processor.py
Show resolved
Hide resolved
spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_service.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: .coderabbit.yaml
Files selected for processing (2)
- spiffworkflow-backend/src/spiffworkflow_backend/api.yml (2 hunks)
- spiffworkflow-backend/src/spiffworkflow_backend/background_processing/celery_tasks/process_instance_task_producer.py (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- spiffworkflow-backend/src/spiffworkflow_backend/api.yml
Additional comments: 1
spiffworkflow-backend/src/spiffworkflow_backend/background_processing/celery_tasks/process_instance_task_producer.py (1)
- 7-8: The import statements for
ApiErrorandProcessInstanceExecutionModehave been added to support the new functionality related to asynchronous execution. These changes are necessary for the implementation of the new feature and do not introduce any issues.
...c/spiffworkflow_backend/background_processing/celery_tasks/process_instance_task_producer.py
Show resolved
Hide resolved
This adds the ability to pass an execution_mode query param into the api calls for task_submit and process_instance_run.
API calls:
/tasks/{process_instance_id}/{task_guid}[PUT]/process-instances/{modified_process_model_identifier}/{process_instance_id}/run[POST]Valid options are:
execution_modequery param can be omitted)Summary by CodeRabbit
execution_modequery parameter to API endpoints for process instances and tasks, allowing users to specify "synchronous" or "asynchronous" execution modes.execution_modeparameter, enhancing control over task and process instance execution.