Support for the maxlag parameter (with retries) in API requests (fixes #92)#112
Support for the maxlag parameter (with retries) in API requests (fixes #92)#112waldyrious merged 5 commits intohamstar:masterfrom
Conversation
|
LGTM, but I'll let this one simmer for a couple days, in case @addshore wants to chime in. |
|
Oh good, because I hadn't requested a review yet. Several more commits with the actual mechanism will follow today before the PR is ready for review. 😉 |
|
As part of centralizing API calls and their response processing in |
30b58e0 to
9013d50
Compare
waldyrious
left a comment
There was a problem hiding this comment.
I left a bunch of mostly inconsequential suggestions inline, but the one in line 215 of Wikimate.php may actually be a bug. Please take a look.
|
All comment improvements and two code fixes applied locally. Just wondering how to commit them: one bulk commit is easy, splitting all changes into separate commits and rebasing all with the previous ones would be more cumbersome, but the end result probably cleaner. Any advice? |
Splitting and rebasing is preferred in ideal conditions, but given the delay my late review has caused already, I don't feel it's inadequate to apply them in a separate commit. It's not like the past git history of the project is pristine anyway :) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Haha, no worries, it was fun! In fact, don't tell anyone yet, but I've already bought styrofoam balls to try and make my own version of the "Git for Ages 4 And Up" video 😉
Yep, that appears to be the (topologically) correct commit graph :) Should be ready to merge. I'll go ahead and do it to not stall this any longer, and will also collapse the git-related comments above to clean up the thread a bit. |
|
On a separate note, just to be sure — did you get my latest emails about the rendered docs? |
Don't have to tell, as you just did in a public comment there. 😉 But another contribution to understanding git should be worthwhile, so good luck with that. 👍
Thanks, phew, finally. 😅
Yes, but back-burnered doing anything in that area for two reasons. |
This parameter insures that API requests can be dropped after the specified number of seconds by a replicated wiki server cluster (such as Wikipedia) if it is strained too much. If a maxlag response is returned, Wikimate delays (sleeps) for a given number of seconds before retrying the request. It can do this indefinitely, or for a limited number of retries. See the documentation at https://www.mediawiki.org/wiki/Manual:Maxlag_parameter
For the implementation I looked around at other PHP MediaWiki libraries for inspiration, and in the bot functions table (linked from here) found an abandoned library called Pillar. Here the request class offered a useful example of handling the lag.