Github rate limit

[mgwidmann]

Currently theres no handling of the rate limit in this project. I regularly hit the rate limit (usually at the end of the hour) and would like to hear your thoughts on how to handle this properly.

Heres the exception I get when that happens:

[error] Task #PID<0.516.0> started from #PID<0.515.0> terminating
** (FunctionClauseError) no function clause matching in Tentacat.process_stream/1
    lib/tentacat.ex:102: Tentacat.process_stream({403, %{"documentation_url" => "https://developer.github.com/v3/#rate-limiting", "message" => "API rate limit exceeded for mgwidmann."}})
    (elixir) lib/stream.ex:1099: Stream.do_resource/5
    (elixir) lib/enum.ex:1486: Enum.reduce/3
    (elixir) lib/enum.ex:2248: Enum.to_list/1

[duksis]

not sure what you mean by "no handling of the rate limit"
What is your usecase? Tentacat is a simple wrapper for the Github API and most likely you can avoid hitting the rate limits by rethinking your use case or its implementation.

one thing I could think of improving on tentacats side is trying to implement https://developer.github.com/v3/#conditional-requests but I'm not sure whether that would be a solution for your usecase.

[mgwidmann]

That would most likely greatly improve my rate limiting... I'd give a 👍 that

What I mean is that I get that exception when I hit the limit. Perhaps instead of returning the data, Tentacat should return a {:ok, data} or {:error, reason} kind of tuples, and then have a corresponding ! method which does what the current implementation now does. This way if I'm unsure if I'll hit the limit i can choose to handle it, right now I have no choice but to rescue the exception.

[joelmccracken]

I implemented a caching layer around Tentacat for this reason.

The code for the cache layer is here:

https://github.com/thinkthroughmath/dev_wizard/blob/master/lib/dev_wizard/github_gateway/cache.ex

Usage is here:

https://github.com/thinkthroughmath/dev_wizard/blob/master/lib/dev_wizard/github_gateway.ex#L34

This solved the problem for us.

[edgurgel]

There's a discussion about adding a cache store support to httpoison edgurgel/httpoison#172 . Maybe this could help us all!

[ghost]

Hmm. The rate limit is not a problem, imho. The lack of exposure of the rate limit headers in the response is a problem - i.e.

X-RateLimit-Limit: 5000
X-RateLimit-Remaining: 4996
X-RateLimit-Reset: 1372700873

The ETag and Last-Modified headers are also important when working with Github.

ETag: "644b5b0155e6404a9cc4bd9d8b1ae730"
Last-Modified: Thu, 05 Jul 2012 15:31:30 GMT

I guess this will break the world in terms of case statements with the signature changing from :

@type response :: {integer, any} | :jsx.json_term 

to :

@type response :: {integer, any,  HTTPoison.Response.t  } | 
{:ok, :jsx.json_term, HTTPoison.Base.headers }

There is also a Github Ratelimit API - https://developer.github.com/v3/rate_limit/ but hitting it all the time and dealing with the coordination is gonna be a pita when the info is already being returned in each individual request.

Any thoughts? I mean, in the worst case I can just do raw requests for everything but I'm sure anyone that hits github with a lot of requests is gonna have a similar need (they rate-limit very early, especially when un-authenticated)

Full disclosure, I'm writing a CI server.

[ghost]

Quick experiment, changing the process_response implementation - 116 of 198 tests fail) - looks like it does break the world 😂

  @spec process_response(HTTPoison.Response.t) :: response
  def process_response(%HTTPoison.Response{status_code: 200, body: body, headers: headers}), do: {:ok, body, headers}
  def process_response(%HTTPoison.Response{status_code: status_code, body: body }), do: { status_code, body }

[ghost]

@edgurgel - any suggestions to work around this feature? Tentacat 2.0? Fork it. Submit a massive p/r ?

[edgurgel]

@bryanhuntesl, well Tentacat is not 1.0.0 yet so we can do breaking changes! I wonder if we should just always return the raw HTTPoison response as the third argument. It will avoid issues in the future and simplify some weird use cases. What do you think?

[ghost]

Sounds good to me. I can't think of a better way to do it

…

On Sun, 26 Nov 2017 7:26 PM Eduardo Gurgel, ***@***.***> wrote: @bryanhuntesl <https://github.com/bryanhuntesl>, well Tentacat is not 1.0.0 yet so we can do breaking changes! I wonder if we should just always return the raw HTTPoison response as the third argument. It will avoid issues in the future and simplify some weird use cases. What do you think? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#84 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/Aego9tYAC5fHQl9UwORTz-wJFrpZcgXsks5s6bt6gaJpZM4IeSCd> .

[edgurgel]

And then it's kinda of "easy" to build a module to check for the rate limiting based on the HTTPoison.Response? Something like: Tentacat.RateLimit.allowed?(response) and/or Tentacat.RateLimit.remaining(response)

[ghost]

Nice. I'm thinking of a process with an ets table or a process per token. I'm working my way through the unit tests btw. Will create a branch and p/r for review soon.

[ghost]

@mgwidmann - see #132 - I'll share whatever I build on top of that - most likely a coordinator per token