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

feat(#575): retry failed commands #590

Closed
wants to merge 1 commit into from
Closed

Conversation

m5r
Copy link
Member

@m5r m5r commented Nov 29, 2023

Description

This one catches failures a level higher compared to #583, it retries a command as a whole instead of the more granular HTTP request that #583 does

#575

Code review items

  • Readable: Concise, well named, follows the style guide, documented if necessary.
  • Documented: Configuration and user documentation on cht-docs
  • Tested: Unit and/or integration tests where appropriate
  • Backwards compatible: Works with existing data and configuration. Any breaking changes documented in the release notes.

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

@m5r m5r marked this pull request as ready for review November 29, 2023 22:47
@m5r
Copy link
Member Author

m5r commented Nov 29, 2023

@kennsippell could you share your take as to what would be the better and more robust approach between this PR and #583 to make sure cht-conf stays reliable for deployments like Muso Mali

@m5r m5r changed the title Alternative retry failed commands feat(#575): retry failed commands Nov 29, 2023
@jkuester
Copy link
Contributor

jkuester commented Dec 1, 2023

I concur with Gareth's comment that #583 is probably a more viable fix than this PR. One additional point to consider is that this approach would not be super helpful when an action might involve a large amount of HTTP requests. For example, if you have 100+ forms, the upload-app-forms action is going to make a ton of HTTP requests. Retrying at the action level is not going to be very helpful in getting the action to complete successfully....

@m5r m5r closed this Dec 5, 2023
@m5r m5r mentioned this pull request Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants