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

Change max retries to retry duration, refactor settings for consistency #381

Merged
merged 5 commits into from
Jan 9, 2024

Conversation

dbwiddis
Copy link
Member

@dbwiddis dbwiddis commented Jan 7, 2024

Description

  1. Changes retries based on steps to a duration-based retry that will time out.
  2. Moves the max workflows and request retry settings to the settings object for consistency with all the other settings.
  3. Applies the request timeout setting to all searches. It was only used in the max workflow search.

Issues Resolved

Fixes #330
Fixes #338

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

codecov bot commented Jan 7, 2024

Codecov Report

Attention: 25 lines in your changes are missing coverage. Please review.

Comparison is base (fea3e6e) 72.49% compared to head (eafefd4) 72.80%.

Files Patch % Lines
...ework/transport/CreateWorkflowTransportAction.java 64.51% 19 Missing and 3 partials ⚠️
...owframework/rest/AbstractSearchWorkflowAction.java 66.66% 1 Missing ⚠️
...h/flowframework/rest/RestCreateWorkflowAction.java 75.00% 1 Missing ⚠️
...mework/workflow/AbstractRetryableWorkflowStep.java 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #381      +/-   ##
============================================
+ Coverage     72.49%   72.80%   +0.30%     
- Complexity      601      602       +1     
============================================
  Files            76       75       -1     
  Lines          3018     3008      -10     
  Branches        235      234       -1     
============================================
+ Hits           2188     2190       +2     
+ Misses          726      716      -10     
+ Partials        104      102       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dbwiddis dbwiddis changed the title Change max retries to retry duration Change max retries to retry duration, remove timeout setting Jan 7, 2024
@dbwiddis dbwiddis changed the title Change max retries to retry duration, remove timeout setting Change max retries to retry duration, refactor settings for consistency Jan 8, 2024
@dbwiddis dbwiddis force-pushed the retry-settings branch 2 times, most recently from 50733ed to ad2a989 Compare January 8, 2024 17:11
Copy link
Member

@owaiskazi19 owaiskazi19 left a comment

Choose a reason for hiding this comment

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

Overall LGTM with one question

@dbwiddis dbwiddis merged commit a6fb532 into opensearch-project:main Jan 9, 2024
21 checks passed
@dbwiddis dbwiddis deleted the retry-settings branch January 9, 2024 18:10
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 9, 2024
…cy (#381)

* Change max retries to retry duration

Signed-off-by: Daniel Widdis <[email protected]>

* Move max workflows setting update consumer to settings class

Signed-off-by: Daniel Widdis <[email protected]>

* Move workflow timeout setting update consumer to settings class

Signed-off-by: Daniel Widdis <[email protected]>

* Use timeout in other search requests

Signed-off-by: Daniel Widdis <[email protected]>

* Improve test coverage

Signed-off-by: Daniel Widdis <[email protected]>

---------

Signed-off-by: Daniel Widdis <[email protected]>
(cherry picked from commit a6fb532)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
dbwiddis pushed a commit that referenced this pull request Jan 9, 2024
…s for consistency (#390)

Change max retries to retry duration, refactor settings for consistency (#381)

* Change max retries to retry duration



* Move max workflows setting update consumer to settings class



* Move workflow timeout setting update consumer to settings class



* Use timeout in other search requests



* Improve test coverage



---------


(cherry picked from commit a6fb532)

Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
joshpalis pushed a commit to joshpalis/opensearch-ai-flow-framework that referenced this pull request Jan 12, 2024
…s for consistency (opensearch-project#390)

Change max retries to retry duration, refactor settings for consistency (opensearch-project#381)

* Change max retries to retry duration



* Move max workflows setting update consumer to settings class



* Move workflow timeout setting update consumer to settings class



* Use timeout in other search requests



* Improve test coverage



---------


(cherry picked from commit a6fb532)

Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport PRs to 2.x branch
Projects
None yet
4 participants