Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API calls counter #7

Closed
payne911 opened this issue Feb 12, 2021 · 10 comments
Closed

API calls counter #7

payne911 opened this issue Feb 12, 2021 · 10 comments
Labels
enhancement New feature or request

Comments

@payne911
Copy link
Collaborator

payne911 commented Feb 12, 2021

It could be interesting to provide a real-time counter of the amount of GitHub API calls:

  • amount issued for the current query
  • amount left on the current token
  • approximated amount ?

The approximated amount would be displayed at some point before the query begins.

estimate = 1 + total_forks + (total_forks / 100) + (sub_forks_probability * total_forks)

A warning could be issued if that value exceeds the maximum amount of API calls that can be made within an hour with the current token.

@payne911 payne911 changed the title API calls counter API calls and Forks counter Feb 12, 2021
@payne911 payne911 added enhancement New feature or request website labels Feb 12, 2021
@payne911 payne911 changed the title API calls and Forks counter API calls counter Feb 15, 2021
@payne911
Copy link
Collaborator Author

payne911 commented Feb 26, 2021

image

Thanks to bulma-badge !


And a GIF, jus' cuz' :

calls_counter

@payne911
Copy link
Collaborator Author

@stdedos you seem to be knowledgeable about GraphQL: would you mind expanding a bit more on your concerns ?

The idea behind this "calls counter" feature was to provide a bit more visibility into the whole "Rate Limit Exceeded" error. Any ideas on how to adapt this feedback ?

@stdedos
Copy link

stdedos commented Feb 26, 2021

First things first: I am using a dedicated token, and did again the https://useful-forks.github.io/?repo=dbeaver%2Fdbeaver

image

$  curl -i -u usr   -H "Accept: application/vnd.github.v3+json"   https://api.github.com/rate_limit
Enter host password for user 'usr':
HTTP/2 200
server: GitHub.com
date: Fri, 26 Feb 2021 17:13:28 GMT
content-type: application/json; charset=utf-8
content-length: 827
x-ratelimit-limit: 5000
x-ratelimit-remaining: 3571
x-ratelimit-reset: 1614363065
x-ratelimit-used: 1429
cache-control: no-cache
x-oauth-scopes: public_repo
x-accepted-oauth-scopes:
x-github-media-type: github.v3; format=json
access-control-expose-headers: ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, Deprecation, Sunset
access-control-allow-origin: *
strict-transport-security: max-age=31536000; includeSubdomains; preload
x-frame-options: deny
x-content-type-options: nosniff
x-xss-protection: 1; mode=block
referrer-policy: origin-when-cross-origin, strict-origin-when-cross-origin
content-security-policy: default-src 'none'
vary: Accept-Encoding, Accept, X-Requested-With
x-github-request-id: DE23:13A33:1E5F1C:1EEACA:60392C38

{
  "resources": {
    "core": {
      "limit": 5000,
      "used": 1429,
      "remaining": 3571,
      "reset": 1614363065
    },
    "search": {
      "limit": 30,
      "used": 0,
      "remaining": 30,
      "reset": 1614359668
    },
    "graphql": {
      "limit": 5000,
      "used": 19,
      "remaining": 4981,
      "reset": 1614360272
    },
    "integration_manifest": {
      "limit": 5000,
      "used": 0,
      "remaining": 5000,
      "reset": 1614363208
    },
    "source_import": {
      "limit": 100,
      "used": 0,
      "remaining": 100,
      "reset": 1614359668
    },
    "code_scanning_upload": {
      "limit": 500,
      "used": 0,
      "remaining": 500,
      "reset": 1614363208
    }
  },
  "rate": {
    "limit": 5000,
    "used": 1429,
    "remaining": 3571,
    "reset": 1614363065
  }
}

It's very honorable that you don't exhaust my limit, but I am worried that maybe not everything is what it seems.

Also, you have an off-by-one error 😀

@stdedos
Copy link

stdedos commented Feb 26, 2021

@stdedos you seem to be knowledgeable about GraphQL: would you mind expanding a bit more on your concerns ?

It's not a concern, it's merely what GH documentation says: https://docs.github.com/en/graphql/overview/resource-limitations#rate-limit

Why are the API rate limits different? With GraphQL, one GraphQL call can replace multiple REST calls. A single complex GraphQL call could be the equivalent of thousands of REST requests. While a single GraphQL call would fall well below the REST API rate limit, the query might be just as expensive for GitHub's servers to compute.

(also, I am by far not knowledgeable in GraphQL: people try to teach me, and I valiantly defend!)

The idea behind this "calls counter" feature was to provide a bit more visibility into the whole "Rate Limit Exceeded" error. Any ideas on how to adapt this feedback ?

  • First of all, you could avoid exhausting a user's limit 😀
  • Give a no-panic option to abort the scanning. If:
    • user's limit is running out (but with that, you also need to expose user's current limit),
    • a user made a mistake in commissioning a big search,
    • a user is fine trading off partial results to still have available API calls,
  • if you immediately receive a HTTP/2 403 Forbidden request, you can immediately inform the user what the error is, and when he should retry

With all of that, I would personally change this:

JQ_TOTAL_CALLS.html(total + " calls");

to something:

JQ_TOTAL_CALLS.html(`${issued} calls, ${user_allowance.remaining}/${user_allowance.limit} available`);

var user_allowance = undefined;

function body_onload() {
  $(document).ready(get_rate_limit());
}

function get_rate_limit() {
  $.ajax({
    url: "https://api.github.com/rate_limit",
    type: "GET",
    beforeSend: xhr => {
      xhr.setRequestHeader('Accept', 'application/vnd.github.v3+json');
      if (user_token) {
        xhr.setRequestHeader('Authorization', `token ${user_token}`);
      }
    },
    success: function(result) {
      console.log(result);
      $("body").append(`${result.rate.used}/${result.rate.remaining}/${result.rate.limit}`);
      user_allowance = result.rate;
    },
    error: function(error) {
      console.error(error);
    }
  });
}

// Contains debug statements

which then the user could refresh manually, or it's refreshed before a new search is issued. If that happens, issued_calls become 0 and available_calls is re-calculated.

In order not to loose the stats, when fetching finishes, you can pad your result message with "... which took xyz REST API calls". (so if/when the user refreshes the issued/available calls info text, he won't inadvertently loose the issues calls no, if it is of interest to him somehow)

Also ... keep in mind that I am very opinionated about how a UI should look like. Take my words with a grain of salt if necessary 😉

@stdedos
Copy link

stdedos commented Feb 26, 2021

Also also ... If you decide to start with conditional requests, keep in mind that you should count your requests post-flight instead of pre-flight:

function send(request) {
ONGOING_REQUESTS_COUNTER++;
TOTAL_API_CALLS_COUNTER++;
setApiCallsLabel(TOTAL_API_CALLS_COUNTER);
request.send();
}

  • 304s don't count against the limit, and
  • "Maybe" a request will fail to fly. If it never reaches the API, it does not affect your limits 😉

https://docs.github.com/en/rest/guides/getting-started-with-the-rest-api#conditional-requests

@payne911
Copy link
Collaborator Author

payne911 commented Feb 27, 2021

It's very honorable that you don't exhaust my limit, but I am worried that maybe not everything is what it seems.

Hell, you are right! I never questioned the initial code that I had forked :

    } else if (xhr.status === 403) {
        console.warn('Looks like the rate-limit was exceeded.');
    }

The actual error is this one:

HTTP/1.1 403 Forbidden
Content-Type: application/json; charset=utf-8
Connection: close

{
  "message": "You have triggered an abuse detection mechanism and have been temporarily blocked from content creation. Please retry your request again later.",
  "documentation_url": "https://docs.github.com/rest/overview/resources-in-the-rest-api#abuse-rate-limits"
}

I somehow never really paid attention to the fact that I was getting this error way before actually reaching the limit. Thanks for looking into it and pointing it out. On my end, it happened after 477 requests.

From what I've gathered, it's not like GitHub has a fixed number hardcoded somewhere which determines how many requests per second is fine. They seem to calculate that based on how compute-intensive requests are (which is also how they are trying to do the rate limiting with GraphQL).

Part of the API response's headers is retry-after: 60 when the thing errors out, so I guess that's what should be used. However, it's a bit sad to have to ask the users to wait that long before continuing.


Also, you have an off-by-one error 😀

I actually tried to look into it but only ended up noticing even weirder behavior. I tried a query for payne911/PieMenu (which is the default for empty queries) and then checked onto my limit:

curl -i -v -H "Authorization: token THE_SAME_TOKEN" -H "Cache-Control: no-cache"  https://api.github.com/rate_limit?SomethingDiffToForceNoCache

The tool said 6, and the curl too. Plus, I went ahead and looked at the Network debugging tab and counted them by hand (everything is duplicated because somehow OPTIONS are called before each GET, and I'm noticing that is probably increasing the query time by at least 33%):

image

Then I tried a bigger repository (EsotericSoftware/kryonet) and the 414 calls was right too.

Anyways, the one-off is probably related to failed requests: on my end I do increment the counter because the request was sent, but the GitHub API doesn't update its counter on failed requests.


First of all, you could avoid exhausting a user's limit 😀

How so? 😀

Give a no-panic option to abort the scanning.

That was already planned, although I'm not sure when I'll have time to work on that.

keep in mind that I am very opinionated about how a UI should look like

I highly appreciate constructive criticism.


In conclusion... so much work, for so many unknowns. I really wish GitHub was more open about how I should handle this kind of case. It's not clear to what extent I should slow down my requests, nor is it clear if I would indeed benefit from swapping to GraphQL.

@stdedos
Copy link

stdedos commented Feb 27, 2021

It's very honorable that you don't exhaust my limit, but I am worried that maybe not everything is what it seems.

Hell, you are right! I never questioned the initial code that I had forked :

    } else if (xhr.status === 403) {
        console.warn('Looks like the rate-limit was exceeded.');
    }

} else if (xhr.status === 403) {
console.warn('Looks like the rate-limit was exceeded.');
setMsg(UF_MSG_API_RATE);
} else {

The actual error is this one:

HTTP/1.1 403 Forbidden
Content-Type: application/json; charset=utf-8
Connection: close

{
  "message": "You have triggered an abuse detection mechanism and have been temporarily blocked from content creation. Please retry your request again later.",
  "documentation_url": "https://docs.github.com/rest/overview/resources-in-the-rest-api#abuse-rate-limits"
}

TaDaaaaaaaa 😀


Also, you have an off-by-one error 😀

Anyways, the one-off is probably related to failed requests: on my end I do increment the counter because the request was sent, but the GitHub API doesn't update its counter on failed requests.

#7 (comment)


First of all, you could avoid exhausting a user's limit 😀

How so? 😀

Count the approximate amount and compare that with the rate.remaining.
If that is >70-80%, ask "Are you sure?"


Give a no-panic option to abort the scanning.

That was already planned, although I'm not sure when I'll have time to work on that.

I don't see a complexity here 😕

Allow your button to be pressed while scanning, turn it red, onclick() => ABORTED = true, disable the button again, and here

function send(request) {
ONGOING_REQUESTS_COUNTER++;
TOTAL_API_CALLS_COUNTER++;
setApiCallsLabel(TOTAL_API_CALLS_COUNTER);
request.send();
}

	  ...
	  if (ABORTED) {
		  return;
	  }
	  ...

Then, here

if (allRequestsAreDone()) {
sortTable();
enableQueryFields();
}

ABORTED = undefined

(or whatever is js-appropriate)

I can PR this sometime tomorrow if you want; but I don't know if/how I will be able to test this.


In conclusion... so much work, for so many unknowns. I really wish GitHub was more open about how I should handle this kind of case. It's not clear to what extent I should slow down my requests, nor is it clear if I would indeed benefit from swapping to GraphQL.

❤️

@payne911
Copy link
Collaborator Author

That was already planned, although I'm not sure when I'll have time to work on that.

I don't see a complexity here 😕

Just to explain a bit the situation: I've been spending a bit too much time working on this personal project rather than studying for my upcoming interviews. ;)

@stdedos
Copy link

stdedos commented Feb 27, 2021

Just to explain a bit the situation: I've been spending a bit too much time working on this personal project rather than studying for my upcoming interviews. ;)

😄😄😄😄😄

I have an interview deliverable due tomorrow 🤣

Good luck!!!! 😄

@payne911
Copy link
Collaborator Author

payne911 commented Mar 5, 2021

@stdedos if you feel like sending a PR, I've opened #16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants