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.

Sign up for GitHub

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

Jump to bottom

APP-2821: Implement a configurable retry mechanism #232

Merged
merged 21 commits into from
Sep 24, 2020
Merged

APP-2821: Implement a configurable retry mechanism #232

merged 21 commits into from
Sep 24, 2020

Conversation

symphony-elias
Copy link
Contributor

@symphony-elias symphony-elias commented Sep 17, 2020 •
edited
Loading

Ticket

APP-2821

Description

Goal is to provide a generic retry mechanism for http calls. Refactored DFv1 and DFv2. Will extend this to all services.
Also contains code reformatting (according to the .editorconfig file : 2 spaces for indentation).

Dependencies

None

Checklist

  • Referenced a ticket in the PR title and in the corresponding section
  • Filled properly the description and dependencies, if any
  • Unit tests updated or added
  • Javadoc added or updated

@symphony-elias symphony-elias changed the title APP-2821 APP-2821: Implement a configurable retry mechanism Sep 17, 2020
@symphony-elias symphony-elias marked this pull request as ready for review September 21, 2020 15:29
@symphony-elias symphony-elias requested a review from a team September 21, 2020 15:29
@symphony-youri
Copy link
Contributor

Looks good to me
Perhaps we can also try to run the datafeed locally and try to break in different ways (break auth, cut wifi, shutdown server, etc) and see how it copes and if it keeps running as expected, maybe this manual testing will expose cases we did not think of (and also see if error reporting is good enough to help understand the error_

thibauult
thibauult approved these changes Sep 24, 2020
View reviewed changes
Copy link
Member

@thibauult thibauult left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@symphony-elias symphony-elias merged commit 083b7d7 into finos:master Sep 24, 2020
@symphony-elias symphony-elias deleted the APP-2821 branch October 20, 2020 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@symphony-hong symphony-hong symphony-hong left review comments

@symphony-youri symphony-youri symphony-youri approved these changes

@thibauult thibauult thibauult approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@symphony-elias @symphony-youri @thibauult @symphony-hong