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

Introduce Configurable Upper-Limits/default values for max retries and retry Interval in Error Handling #12787

Open
5 tasks done
kazuhisa-wada opened this issue Jan 16, 2025 · 0 comments · May be fixed by #12788
Open
5 tasks done
Labels
💪 enhancement New feature or request

Comments

@kazuhisa-wada
Copy link
Contributor

Self Checks

  • I have searched for existing issues search for existing issues, including closed ones.
  • I confirm that I am using English to submit this report (我已阅读并同意 Language Policy).
  • [FOR CHINESE USERS] 请务必使用英文提交 Issue,否则会被关闭。谢谢!:)
  • Please do not modify this template :) and fill in all the required fields.

1. Is this request related to a challenge you're experiencing? Tell me about your story.

Enhancement suggestion

This is enhancement request related to error handling of workflow node i.e. http, llm, and tool introduced in v.0.14.2.

I would like to suggest following enhancements:

  • upper limit of max_retries and retry_interval are configurable in each retry-able nodes only if configured
  • default values of retries are configurable in each retry-able nodes only if configured
  • not hardcoded but configurable system-wide default values of retries is applied only if nothing is configured

Background of this request

Current concern

  • HTTP node's default value: retry=enabled, max_retry=3, retry_interval=100ms which should be configurable in each environment because many users simply use these settings as-is, potentially causing redundant unnecessary HTTP retries, however, redundant HTTP retries are in general discouraged depending on the policy of environments where dify deployed. In fact, at least in my company, default retry should be disabled, upper limit of max_retry=10 is too large and dangerous.
  • upper limit of max_retries and retry_interval: LLM node’s upper limit of retry interval (5000ms) can be too short for some scenarios. This is related to the issues I raised here regarding Amazon bedrock where I want to configure retry interval larger. However, current upper limit of retry interval is hardcoded system-wide.

In response to above concerns, I would like to suggest enhancements above.

Concrete implementation approach

I have already implemented this in my local env and will push it later to main repository, please review if it is good, thanks.

  1. to change upper limit of max_retry and retry_interval from hardcode to configured values which are passed from each node initialization. If it's not specified, system default values are loaded from config file. This is implemented inweb/app/components/workflow/nodes/_base/components/retry/retry-on-panel.tsx.
  2. to do 1, in default.ts for each retry-able node(LLM, Node, Tool), fix default value for retry_config property so that retry-on-panel.tsx can access each node's retry default values and upper limit constraints configured in config file.
  3. to do 2, in type.ts in retry component, adding max_retries_upper_bound and retry_interval_upper_bound
  4. to add configuration variables in config/index.ts for both of system-wide default values and each node's values.

Note: no behavior change should happen after this change is applied unless config file is changed intentionally for each nodes.

System default retry config as-is are as follows:

  • retry enable/disable: disable
  • max_retry: 3
  • retry_interval: 1000ms
  • upper limit of max_retry: 10
  • upper limit of retry_interval: 5000

2. Additional context or comments

Described in 1.

3. Can you help us with this feature?

  • I am interested in contributing to this feature.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💪 enhancement New feature or request
Projects
None yet
2 participants