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

fix: add retry on errors for getKeepalivedStats #133

Merged
merged 1 commit into from
Jul 23, 2023
Merged

fix: add retry on errors for getKeepalivedStats #133

merged 1 commit into from
Jul 23, 2023

Conversation

mehdy
Copy link
Owner

@mehdy mehdy commented Jul 23, 2023

Fixes #115
Closes #117 due to duplication

@mehdy mehdy added the bug Something isn't working label Jul 23, 2023
@mehdy mehdy requested a review from clwluvw July 23, 2023 16:35
@mehdy
Copy link
Owner Author

mehdy commented Jul 23, 2023

@clwluvw I'll clean up the other retry implementation if this is ok.

@clwluvw
Copy link
Collaborator

clwluvw commented Jul 23, 2023

@clwluvw I'll clean up the other retry implementation if this is ok.

Yes, it completely makes sense - we just need to remove the retry for file as we are already retrying on top of the func for every possible failure.

Copy link
Collaborator

@clwluvw clwluvw left a comment

Choose a reason for hiding this comment

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

Do you think it would make sense to do the retry where getKeepalivedStats is being called?

@mehdy
Copy link
Owner Author

mehdy commented Jul 23, 2023

It actually makes a ton of sense :D

@mehdy mehdy force-pushed the fix-115 branch 2 times, most recently from 54839de to da00d0b Compare July 23, 2023 20:49
@mehdy mehdy requested a review from clwluvw July 23, 2023 20:54
Signed-off-by: Mehdi Khoshnoodi <mehdy.khoshnoody@gmail.com>
Copy link
Collaborator

@clwluvw clwluvw left a comment

Choose a reason for hiding this comment

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

LGTM!

@clwluvw clwluvw merged commit b8475da into master Jul 23, 2023
6 checks passed
@clwluvw clwluvw deleted the fix-115 branch July 23, 2023 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

keepalived.data and keepalived.stats datas are not synced error
2 participants