Skip to content

feat: add a abstract layer for WorkflowNodeExcetion #18026

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

Merged

Conversation

laipz8200
Copy link
Member

Signed-off-by: -LAN- [email protected]

Summary

close #18025

Tip

Close issue syntax: Fixes #<issue number> or Resolves #<issue number>, see documentation for more details.

Screenshots

Before After
... ...

Checklist

Important

Please review the checklist below before submitting your pull request.

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods

…ions

Simplifies storage methods by eliminating try-except blocks
that logged exceptions and re-raised them without additional
processing. This streamlines the codebase and relies on
upstream exception handling to manage errors effectively.

Signed-off-by: -LAN- <[email protected]>
Introduces configuration for workflow node execution storage with options for 'rdbms', 'hybrid', and 's3'. Updates environment examples and integrates the new configuration class into the feature settings.

Enhances flexibility in storage backend selection.

Signed-off-by: -LAN- <[email protected]>
Replaces WorkflowNodeExecutionStatus with StrEnum for automatic string conversion.
Removes redundant value_of method for cleaner and more maintainable code.

Signed-off-by: -LAN- <[email protected]>
Adds repository interfaces for data access abstraction and a factory
for creating repository instances. Implements initialization of
repositories in the application setup. Enhances modularity and
flexibility by decoupling data access logic from storage mechanisms.

Signed-off-by: -LAN- <[email protected]>
Introduces criteria-based filtering capabilities for workflow
node executions, allowing for more flexible querying. Adds new
methods for retrieving and deleting based on criteria, and
removes abstract base class in favor of Protocol for interface
definition. Updates ordering options for retrieved instances,
enabling better control over query results.

Improves repository interface to better handle domain-specific
concepts at the implementation level, maintaining a clean core
domain model.

Signed-off-by: -LAN- <[email protected]>
Includes TODO comments suggesting a transition to a repository pattern
for database interactions. Provides examples for future implementation
but retains current direct database access.

Facilitates future refactoring towards cleaner code architecture.

Signed-off-by: -LAN- <[email protected]>
Eliminates WorkflowNodeExecutionStorageConfig class to simplify
storage configuration. Updates WorkflowNodeExecutionConfig to
directly define storage backend options.

This change reduces complexity by consolidating settings and
avoiding duplication.

Signed-off-by: -LAN- <[email protected]>
Replaces direct database access with repository pattern for managing workflow node executions, enhancing modularity and maintainability. Updates cache handling to ensure consistency with database operations. Streamlines execution queries and deletions across various services by leveraging the new repository.

Removes outdated TODO comments related to repository migration.

Improves code clarity by removing redundant code blocks.

Relates to performance improvement and codebase modernization efforts.

Signed-off-by: -LAN- <[email protected]>
Eliminates get_by_id and delete_by_criteria methods from
WorkflowNodeExecutionRepository as they are not used anywhere
in the codebase, simplifying the interface and reducing
maintenance overhead.

Signed-off-by: -LAN- <[email protected]>
Introduces SQLAlchemy implementation for WorkflowNodeExecution
repository with CRUD operations, supporting multi-tenancy. Registers
the repository based on RDBMS storage configuration.

Includes unit tests to ensure repository functionality.

Signed-off-by: -LAN- <[email protected]>
Simplifies code by consolidating line breaks in the registration
and error handling for repository storage types. Improves
readability and maintains functionality.

No functionality changes.

Signed-off-by: -LAN- <[email protected]>
Updates repository methods to flush without committing, shifting
transaction responsibility to callers. Adds explicit commit calls
after batch deletions and saves to ensure data persistence.

Improves database operation consistency and control.

Signed-off-by: -LAN- <[email protected]>
Replaces direct database queries with a repository pattern
to fetch workflow node executions. This change enhances
code maintainability and aligns with best practices by
abstracting data access logic into a repository.

No issue or ticket reference provided.

Signed-off-by: -LAN- <[email protected]>
@laipz8200 laipz8200 requested a review from Copilot April 14, 2025 12:56
@laipz8200 laipz8200 requested a review from QuantumGhost April 14, 2025 12:56
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 24 out of 25 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • api/.env.example: Language not supported

@QuantumGhost
Copy link
Collaborator

I'll review the PR later, after completing #16317.

@laipz8200 laipz8200 marked this pull request as ready for review April 15, 2025 10:19
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. 🌊 feat:workflow Workflow related stuff. labels Apr 15, 2025
Introduces OrderConfig dataclass to encapsulate ordering
parameters, enhancing code readability and maintainability.
Updates repository methods to use OrderConfig for ordering,
allowing multiple fields to be specified. Adjusts tests to
reflect these changes for consistency and accuracy.

Signed-off-by: -LAN- <[email protected]>
Removes `get_by_id` and `delete_by_criteria` methods as they
are not used anywhere in the codebase, simplifying the
repository interface and improving maintainability.

Signed-off-by: -LAN- <[email protected]>
Simplifies repository creation by introducing a dedicated method in the RepositoryFactory for workflow node execution. Replaces previous generic repository creation calls with the new method for better clarity and type safety. Removes unnecessary imports and type casting, streamlining the codebase.

This update enhances maintainability and reduces the potential for errors in repository initialization across the application.

Signed-off-by: -LAN- <[email protected]>
Eliminates the WorkflowNodeExecutionCriteria class and related
methods, simplifying the querying process by using direct SQLAlchemy
queries. This change reduces complexity in repository interactions
and enhances code maintainability by removing redundant logic.

Refactors services to use direct queries instead of repository
methods, improving performance and clarity in workflow node
execution management.

Signed-off-by: -LAN- <[email protected]>
@laipz8200 laipz8200 marked this pull request as draft April 16, 2025 12:39
Changes enum class inheritance to StrEnum for consistency
across the codebase. Removes the unused value_of method
to simplify code and reduce redundancy.

Signed-off-by: -LAN- <[email protected]>
Refactors workflow node execution handling to use a session factory
for creating new sessions, eliminating direct session passing and
merging. This change aims to improve session management and prevent
long-running connections.

Updates tests to accommodate session factory usage, ensuring
consistent test behavior.

Signed-off-by: -LAN- <[email protected]>
Replaces repository pattern with direct SQLAlchemy queries
for fetching and saving workflow node executions. Enhances
code simplicity and maintainability by removing redundant
repository usage and leveraging native SQLAlchemy session
methods for database operations.

Signed-off-by: -LAN- <[email protected]>
@laipz8200 laipz8200 requested a review from Copilot April 16, 2025 14:02
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 27 out of 28 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • api/.env.example: Language not supported
Comments suppressed due to low confidence (2)

api/services/clear_free_plan_tenant_expired_logs.py:141

  • Row-by-row deletion of workflow node executions may impact performance when processing large batches. Consider using a bulk delete operation if transactional consistency can be maintained.
for execution in workflow_node_executions:

api/extensions/ext_storage.py:75

  • Removal of the try/except block here eliminates error logging, which might hinder debugging of storage failures; ensure that error tracking is handled elsewhere or consider retaining logging.
self.storage_runner.save(filename, data)

@laipz8200 laipz8200 force-pushed the feat/support-hybrid-storage-for-workflow-node-execution branch 2 times, most recently from 20fbce7 to d4fac76 Compare April 16, 2025 14:27
@laipz8200 laipz8200 force-pushed the feat/support-hybrid-storage-for-workflow-node-execution branch from d4fac76 to ff42e85 Compare April 16, 2025 14:28
Copy link
Collaborator

@QuantumGhost QuantumGhost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Apr 17, 2025
@laipz8200 laipz8200 marked this pull request as ready for review April 17, 2025 03:38
@dosubot dosubot bot added the 📚 documentation Improvements or additions to documentation label Apr 17, 2025
@laipz8200 laipz8200 merged commit 6d9dd31 into main Apr 17, 2025
12 checks passed
@laipz8200 laipz8200 deleted the feat/support-hybrid-storage-for-workflow-node-execution branch April 17, 2025 03:48
@rainsoft
Copy link
Contributor

When step by step workflow node, raise exception: 'Engine' object does not support the context manager protocol

image

image

@hjlarry hjlarry mentioned this pull request Apr 22, 2025
5 tasks
@rainsoft
Copy link
Contributor

rainsoft commented Apr 22, 2025

When trace workflow run log, the same error. @hjlarry

image
image

image

@hjlarry hjlarry mentioned this pull request Apr 22, 2025
5 tasks
@hjlarry
Copy link
Contributor

hjlarry commented Apr 22, 2025

When trace workflow run log, the same error. @hjlarry

see #18534
I tested it works now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 documentation Improvements or additions to documentation 🌊 feat:workflow Workflow related stuff. lgtm This PR has been approved by a maintainer size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhance Storage Abstraction for WorkflowNodeExecution
4 participants