Connection reset by peer occurs

[otamachan]

When running colcon mixin update in CI, it very rarely fails with a "Connection reset by peer" error.
It might be better to add retry for this case as well.

  <urlopen error <urlopen error [Errno 104] Connection reset by peer> (https://raw.githubusercontent.com/colcon/colcon-mixin-repository/master/index.yaml)>
fetching default: https://raw.githubusercontent.com/colcon/colcon-mixin-repository/master/index.yaml

[otamachan]

Maybe different issue but 429: Too many requests also happens.

  - HTTP Error 429: Too Many Requests (https://raw.githubusercontent.com/colcon/colcon-mixin-repository/master/build-testing.mixin)
fetching default: https://raw.githubusercontent.com/colcon/colcon-mixin-repository/master/index.yaml ...

[christophebedard]

It might be better to add retry for this case as well.

Or possibly a delay if there isn't one already. colcon mixin update downloads ~15-20 files in quick succession.

You should also refer to GitHub's info about rate limits and ways to increase the limit, especially in a CI environment: https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api (not entirely sure if a simple file download falls under the REST API, though).

[christophebedard]

Making it retry in case of Connection reset by peer sounds reasonable.

In the case of HTTP error 429 Too Many Requests, it looks like it doesn't handle it and just raises instead of retrying, unlike how it handles 503 Service Unavailable.

Improving any of this would be appreciated 😃

[cottsay]

Would it really be appropriate (or even effective) to react to a Too Many Requests error by automatically making more requests?

EDIT: I jumped too soon, after re-reading your reply I don't think you were actually suggesting we do that.

[christophebedard]

Still a very valid point: there's only so much that can be done there. It could retry after a much longer wait/retry period, but that's still more requests 😅

The better approach is probably to allow users to provide some sort of GitHub authentication to get a higher rate limit, especially for CI environments. What do you think?

[cottsay]

The better approach is probably to allow users to provide some sort of GitHub authentication to get a higher rate limit, especially for CI environments. What do you think?

It's tricky. These are plain URLs - they can even be local file:// URLs. We could add a special case to use GitHub's artifact API (possibly) using a token iff the URL can be parsed as a GitHub-specific URL, but it's absolutely a special case in otherwise generic code.

I'm lukewarm on the idea. There seems to be a big bump in the number of 429s coming out of GitHub lately. Maybe we should sit on it for a bit to see if they persist.

[cottsay]

To be clear: totally onboard for automated retry on a connection reset, though.

[nuclearsandwich]

Once upon a time, you could rely on placing network authentication configuration in .netrc and it could be used generically. Python has support for reading netrc files in the standard library but I think use of it for https requests may need to be manually configured in colcon-mixin.

[MiguelCompany]

I went ahead and added the retry for 429s in #49