handle rate limiter & github server-side errors

[mredolatti]

Two issues were found while using this library:

• Random and isolated 502 errors from github make the whole job fail.
• If the number of API calls exceeds github's limit, the whole job fails.

To deal with these scenarios, simple retry logic was added, and in the case of a rate limiting situation, we wait until the next reset time to move forward with the next retry.

[cmerrick]

Hi @mredolatti, thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes.

[cmerrick]

You did it @mredolatti!

Thank you for signing the Singer Contribution License Agreement.

[luandy64]

Couldn't this be replaced with the backoff library?

[luandy64]

@mredolatti

[mredolatti]

Hi @luandy64 i'm not familiar with that library, but i'll look into it as soon as possible and tidy up the PR.

Thanks and sorry for the delay!

[osterman]

@mredolatti I sumbled on this example: https://github.com/singer-io/tap-exchangeratesapi/blob/master/tap_exchangeratesapi/__init__.py#L38

[mredolatti]

I'll take a look at this tonight and see if I can put the branch up to speed

[henriblancke]

@luandy64@osterman could it be that the backoff library doesn't provide an easy way to use headers like: X-RateLimit-Remaining, X-RateLimit-Reset to determine how long to back off: litl/backoff#38?

[nehiljain]

Are there plans to work on this? @osterman or @mredolatti ? I am having the same issue as well

[henriblancke]

@cmerrick@mredolatti any way we can help to get this over the line?

[osterman]

@nehiljain Sorry - no resources available on outside to put towards it at this time

[KBorders01]

@osterman@mredolatti@luandy64, what needs to happen to merge this PR? It looks like the code that's there already is much better than the current code, which has no retry capability. Also, it doesn't look like backoff will handle the rate limit headers anyway.

This issue is blocking me from using Stitch's hosted Github tap, and instead I have to run it on my own.

[antoine-lizee]

Up! Can we merge this?

[savicbo]

@luandy64 can we merge this PR or is has it been implemented elsewhere? Hitting the GitHub ratelimit is affection our ability to use Stitch as well

[mredolatti]

is this still an issue? i took a look at backoff a while ago and didn't see a straightforward way to use it while relying on the response's headers to actually wait the correct amount of time. Do we need to add that behavior into backoff first and then update this?

[bgreen-litl]

@mredolatti latest backoff (2.0.1) has the backoff.runtime decorator which allows you to introspect the exception or response and yield a wait value based on that

[loeakaodas]

@mredolatti@bgreen-litl basic backoff functionality was added in #143. This PR should probably be closed and a custom wait time functionality can be added in a new PR based on the latest version in master.

[luandy64]

@savicbo@mredolatti@bgreen-litl@antoine-lizee@KBorders01@henriblancke@nehiljain

Correct me if I'm wrong, but if you run v1.10.4 of this tap, there's already logic to handle the rate limit headers thanks to @loeakaodas

tap-github/tap_github/__init__.py

Lines 198 to 207 in 4f7ba58

	defrate_throttling(response):
	ifint(response.headers['X-RateLimit-Remaining']) ==0:
	seconds_to_sleep=calculate_seconds(int(response.headers['X-RateLimit-Reset']))
	ifseconds_to_sleep>MAX_SLEEP_SECONDS:
	message="API rate limit exceeded, please try after {} seconds.".format(seconds_to_sleep)
	raiseRateLimitExceeded(message) fromNone
	logger.info("API rate limit exceeded. Tap will retry the data collection after %s seconds.", seconds_to_sleep)
	time.sleep(seconds_to_sleep)

[KBorders01]

@savicbo@mredolatti@bgreen-litl@antoine-lizee@KBorders01@henriblancke@nehiljain

Correct me if I'm wrong, but if you run v1.10.4 of this tap, there's already logic to handle the rate limit headers thanks to @loeakaodas

tap-github/tap_github/__init__.py

Lines 198 to 207 in 4f7ba58

	defrate_throttling(response):
	ifint(response.headers['X-RateLimit-Remaining']) ==0:
	seconds_to_sleep=calculate_seconds(int(response.headers['X-RateLimit-Reset']))
	ifseconds_to_sleep>MAX_SLEEP_SECONDS:
	message="API rate limit exceeded, please try after {} seconds.".format(seconds_to_sleep)
	raiseRateLimitExceeded(message) fromNone
	logger.info("API rate limit exceeded. Tap will retry the data collection after %s seconds.", seconds_to_sleep)
	time.sleep(seconds_to_sleep)

@luandy64 that is correct, it looks like this issue has been resolved in another PR.