feat: Integrate Workflow Manger V2#278
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant upgrade to the application's workflow management capabilities by integrating a new, more robust Workflow Manager V2, powered by Temporal.io. This new system allows for a more flexible and scalable approach to defining and executing complex workflows using a graph-based model. The changes are implemented with a feature flag, allowing for a phased rollout and ensuring that existing functionalities remain operational. This lays the groundwork for enhanced workflow orchestration and improved handling of business processes. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request integrates a new Workflow Manager v2 based on Temporal, using a feature flag to switch between the existing v1 and the new v2 implementation. The changes are extensive, touching configuration, application bootstrap, task management, and workflow services. The use of a feature flag is a sound strategy for this migration. My review highlights several areas for improvement, particularly in logging practices, error handling, and the completeness of the v2 implementation path. I've identified instances where logging levels are inappropriate, error handling could be more robust, and parts of the v2 implementation appear incomplete or may lead to information loss in the API response. For some identified gaps in the v2 implementation, I've noted that deferral is acceptable if tracked, aligning with repository guidelines.
e6dd861 to
2f39a6b
Compare
- Added configuration option to enable the Workflow Manager V2 in the application. - Updated the TaskManager interface to support callbacks for task completion and updates using the v2 workflow manager. - Implemented logic in the ConsignmentService to handle workflow initialization and retrieval for both v1 and v2 workflows. - Introduced a new JSON template for the Customs workflow. - Enhanced unit tests to accommodate the workflow manager v2 integration.
2f39a6b to
3d3307e
Compare
Summary
This PR brings the integration of Workflow Manger package defined in the https://github.com/OpenNSW/go-temporal-workflow. This package introduces the Node and Edges based graph representation for the workflow management with token passing through the edges to unlock new workflow nodes. And it make use of temporal.io for managing states of the workflow nodes.
Type of Change
Changes Made
Testing
Testing Guidelines
cp .env.exampl .envin the rootUSE_WORKFLOW_MANAGER_V2as,falseto validate v1 workflow manager functioning properly without any breaking./start-dev.sh. This will brings up all the services to validate end-to-end flowAdditional Notes
This only bring the new version of the workflow manager such that previous functionality also exists. You need to setup the boolean environment
USE_WORKFLOW_MANAGER_V2astrueto make use of new version of Workflow Manger packageDeployment Notes
USE_WORKFLOW_MANAGER_V2astrue. Guide available at https://docs.temporal.io/self-hosted-guideLimitations need to be address in future PR
Checklist
Related Issues
No specific issues created for this PR
Screenshots/Demo
N/A (backed change)