search

github / gh-classroom

GitHub Classroom CLI Extension is a powerful and easy-to-use command line tool that enhances the functionality of the GitHub CLI, specifically tailored for educators using GitHub Classroom.
MIT License
75 stars 12 forks source link

Simple retry loop for issue #38 #41

Closed shanep closed 10 months ago

shanep commented 10 months ago

What are you trying to accomplish?

Issue #38 still seems to be popping up quite a bit. In addition to the backend fixes that @RyanHecht mention are being worked on https://github.com/github/gh-classroom/issues/38#issuecomment-1731563468 This adds a very simple retry loop to the base http package with a max retry of 5.

While this does not fix #38, it does help mitigate what the users see to help smooth out their experience. Hopefully with lowering the default per-page to 15 and this simple loop this error can be less intrusive.

What approach did you choose and why?

Just a simple retry loop with a max of 5 retry's.

What should reviewers focus on?

It would be really nice if the RESTClient.Get call returned an error code so we could explicitly check for a 500. As of right now it is embedded in the error string. I guess we could parse the error code out of a string and if you think that is a better approach let me know :)

As of right now this simple approach will execute a retry even if the failure was valid as in the case of the user passing in bad data.

I don't know if we want to add in any pauses between retry's to avoid hitting the API too aggressively :) I would assume that github.com could handle the traffic but I don't have any insight to what the backend is doing.

Thanks for all your efforts :)

zrdaley commented 10 months ago

Thank you for this PR! We are currently working on an internal fix that will hopefully improve the performance of this endpoint and prevent 500s. I'd love to hold off on this PR until those changes are merged.

zrdaley commented 10 months ago

This seems to be working now without retries https://github.com/github/gh-classroom/issues/38#issuecomment-1741110222