Skip to content
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

refactor(variables): replace deprecated 'get_any' with 'get' method #9584

Merged

Conversation

laipz8200
Copy link
Collaborator

@laipz8200 laipz8200 commented Oct 21, 2024

Checklist:

Important

Please review the checklist below before submitting your pull request.

  • Please open an issue before creating a PR or link to an existing issue
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods

Description

  • Removed deprecated get_any method in favor of the get method for variable retrieval across multiple nodes.
  • Improved error handling for variable retrieval by adding checks for variable existence and type validation.
  • Introduced specific error messages for missing or invalid variable types to enhance debugging capabilities.

Fixes

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update, included: Dify Document
  • Improvement, including but not limited to code refactoring, performance optimization, and UI/UX improvement
  • Dependency upgrade

Testing Instructions

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. 🐞 bug Something isn't working 💪 enhancement New feature or request labels Oct 21, 2024
Copy link
Contributor

@bowenliang123 bowenliang123 left a comment

Choose a reason for hiding this comment

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

LGTM. Long-awaited refactoring.

crazywoola
crazywoola previously approved these changes Oct 22, 2024
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Oct 22, 2024
@crazywoola crazywoola self-requested a review October 22, 2024 00:59
- Removed deprecated `get_any` method in favor of the `get` method for variable retrieval across multiple nodes.
- Improved error handling for variable retrieval by adding checks for variable existence and type validation.
- Introduced specific error messages for missing or invalid variable types to enhance debugging capabilities.
@laipz8200 laipz8200 force-pushed the refactor/replace-deprecated-'get_any'-with-'get'-method branch from 0a0d24f to eeecdb0 Compare October 22, 2024 02:16
@crazywoola crazywoola merged commit 8f670f3 into main Oct 22, 2024
9 checks passed
@crazywoola crazywoola deleted the refactor/replace-deprecated-'get_any'-with-'get'-method branch October 22, 2024 02:49
@ChristenJacquottet
Copy link

This refactoring is breaking many workflows because some users were reliant on the previous behavior where input variables could be null. The deprecated get_any method allowed workflows to proceed even with missing or null values, providing fault tolerance. The new get method is stricter and requires all variables to exist, resulting in unexpected failures for workflows that aren't explicitly handling missing inputs.

@laipz8200
Copy link
Collaborator Author

This refactoring is breaking many workflows because some users were reliant on the previous behavior where input variables could be null. The deprecated get_any method allowed workflows to proceed even with missing or null values, providing fault tolerance. The new get method is stricter and requires all variables to exist, resulting in unexpected failures for workflows that aren't explicitly handling missing inputs.

You are right. We allow the Optional variable in the START Node to be empty, but we don’t permit misreferenced variables in other cases. This refactoring is necessary to provide a more manageable and optimized user experience in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working 💪 enhancement New feature or request lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants