Sync: Try to fix the GitHub abuse mechanism coping code

[ewels]

• Fixes main bug that the default value for the Retry-After header was an int and not a str
• Make the default wait random, between 10 - 60 seconds
• Some tweaks, print the headers in more log messages, make them prettier.

Fixes #970

PR checklist

• This comment contains a description of changes (with reason)
• CHANGELOG.md is updated
• If you've fixed a bug or added code that should be tested, add tests!
• Documentation in docs is updated

[codecov]

Codecov Report

Merging #978 (a21fa21) into dev (0ce2c58) will decrease coverage by 0.05%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##              dev     #978      +/-   ##
==========================================
- Coverage   69.27%   69.21%   -0.06%     
==========================================
  Files          34       34              
  Lines        4250     4256       +6     
==========================================
+ Hits         2944     2946       +2     
- Misses       1306     1310       +4     

Impacted Files	Coverage Δ
nf_core/sync.py	51.50% <33.33%> (-0.49%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86d83a3...a21fa21. Read the comment docs.

[ewels]

Started writing new tests for this but struggled, as if it's working it just sits in a while loop forever, trying again and again. Not sure how to test for that behaviour?!

[KevinMenden]

Hmm maybe we just set a maximum number of tries instead of a while True loop?

For instance, could say try maximum 10 times and if it didn't work the first 5 times, then wait for 2h or so.
I guess it's fair to assume that if it doesn't work after 10 times and waiting for some hours, it probably never will.

We assign random slots to the sync jobs, which we sample uniformly. So we would have slots 5, 10, 15, 20, ... mins after calling the job. This way the jobs should be spread more or less evenly among the time slots and wouldn't run all at once. But maybe that's a bit too complicated and we just try again until it works 😁

[ewels]

We assign random slots to the sync jobs, which we sample uniformly. So we would have slots 5, 10, 15, 20, ... mins after calling the job. This way the jobs should be spread more or less evenly among the time slots and wouldn't run all at once. But maybe that's a bit too complicated and we just try again until it works 😁

That's sort of what I did here, right? Except with a random number of seconds, maximum one minute. I don't expect that we will need such a large delay between calls to be honest - the example in the docs suggests that the header might say to wait 30 seconds. I think that's more the order of magnitude.

The 403 error code is fairly specific and the actions job will eventually time out after 6 hours (??) or so anyway.

[KevinMenden]

Hmm yeah you're right probably that's enough 👍
Ah okay well in that case that other comment doesn't make sense either. Seems like it should do the trick then.

[ewels]

And hopefully GitHub will tell us how long to wait in the return header anyway, which would be the best way to do it. I wasn't able to test that without abusing GitHub myself, so we'll have to just see...

Happy to merge if you can give a ✅ 😄

[KevinMenden]

👌