scylladb / argus

Apache License 2.0
4 stars 11 forks source link

Getting error while trying to add a GH issue link #435

Open mykaul opened 3 months ago

mykaul commented 3 months ago

I'm getting: API Error while submitting an issue on a test run. Message: Error getting issue state: Response: HTTP 404

I have no idea why. I'm adding to https://argus.scylladb.com/test/5d2da105-5a28-4320-9d86-68b12bae75be/runs?additionalRuns%5B%5D=8960783f-e3c4-4006-9591-359e0d7307cb the bug https://github.com/scylladb/scylla-enterprise/issues/4626

Could be that I'm not logged in to GH in Argus or what not - so it should just let me add the link.

soyacz commented 3 months ago

I added it. Indeed it should let you add the link even without full access.

k0machi commented 3 months ago

The issue here is that Argus tries to get the initial issue state to save - we can probably omit that stage, but it would require adding fallbacks and updates to the stub issue like that.

soyacz commented 3 months ago

The issue here is that Argus tries to get the initial issue state to save - we can probably omit that stage, but it would require adding fallbacks and updates to the stub issue like that.

or use gh api to get that (we shouldn't reach quota if done rarely)

k0machi commented 3 months ago

The issue here is that Argus tries to get the initial issue state to save - we can probably omit that stage, but it would require adding fallbacks and updates to the stub issue like that.

or use gh api to get that (we shouldn't reach quota if done rarely)

I believe that's already what's happening, it's just we are doing it from the user's token and not from app token.

soyacz commented 3 months ago

The issue here is that Argus tries to get the initial issue state to save - we can probably omit that stage, but it would require adding fallbacks and updates to the stub issue like that.

or use gh api to get that (we shouldn't reach quota if done rarely)

I believe that's already what's happening, it's just we are doing it from the user's token and not from app token.

I mean using app token instead user for this one

k0machi commented 2 months ago

The issue here is that Argus tries to get the initial issue state to save - we can probably omit that stage, but it would require adding fallbacks and updates to the stub issue like that.

or use gh api to get that (we shouldn't reach quota if done rarely)

I believe that's already what's happening, it's just we are doing it from the user's token and not from app token.

I mean using app token instead user for this one

IIRC I did this way because application was not inside scylladb org at the time, which meant user token had more visibility into scylla repos than app token, but since app is now inside the org, there should be no problem changing token to instead use the application one.

soyacz commented 2 months ago

since app is now inside the org, there should be no problem changing token to instead use the application one.

Let's use it and ditch 'Disable full repo access` checkbox

fruch commented 1 week ago

@k0machi is what @soyacz suggested here was done already ?

fruch commented 6 days ago

So we idendify the the failure is only when adding private repo issue, and we shouldn't fail on it

@k0machi would suggest the full example of the fix

k0machi commented 4 days ago

Per discussion the issue here is that when you submit an issue, a request to github is issued to get the issue title - this can be made optional and instead updated when another user with full access tries to read that issue.

Moving from user tokens to an app token brings another issue - rate limits for API requests, with a single token they can be hit pretty quickly as the default limit is pretty low at around 1000 request if I remember correctly.

fruch commented 4 days ago

Per discussion the issue here is that when you submit an issue, a request to github is issued to get the issue title - this can be made optional and instead updated when another user with full access tries to read that issue.

Moving from user tokens to an app token brings another issue - rate limits for API requests, with a single token they can be hit pretty quickly as the default limit is pretty low at around 1000 request if I remember correctly.

https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28

It's 5000-15000 requests per hour. We don't want to go there, we'll for sure gonna hit it

mykaul commented 4 days ago

Per discussion the issue here is that when you submit an issue, a request to github is issued to get the issue title - this can be made optional and instead updated when another user with full access tries to read that issue. Moving from user tokens to an app token brings another issue - rate limits for API requests, with a single token they can be hit pretty quickly as the default limit is pretty low at around 1000 request if I remember correctly.

https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28

It's 5000-15000 requests per hour. We don't want to go there, we'll for sure gonna hit it

Say 6000/hour, that's 100/min, 1.5/sec - how many GH issues do we need to handle concurrently? How many do we even have in the system?

k0machi commented 4 days ago

Per discussion the issue here is that when you submit an issue, a request to github is issued to get the issue title - this can be made optional and instead updated when another user with full access tries to read that issue. Moving from user tokens to an app token brings another issue - rate limits for API requests, with a single token they can be hit pretty quickly as the default limit is pretty low at around 1000 request if I remember correctly.

https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28 It's 5000-15000 requests per hour. We don't want to go there, we'll for sure gonna hit it

Say 6000/hour, that's 100/min, 1.5/sec - how many GH issues do we need to handle concurrently? How many do we even have in the system?

Large releases like scylla master have a lot of issues, and if someone decides to page through all of them on the release dashboard page - they might contribute to this limit.

However I admit that I think that hitting the limit is not as easy anymore - issue widget is loaded on demand and is paged (each page caching the result until a hard refresh), so this limit should be fairly hard to hit (unless of course 50 people decide to open 100 issues all in the same hour)

fruch commented 4 days ago

Per discussion the issue here is that when you submit an issue, a request to github is issued to get the issue title - this can be made optional and instead updated when another user with full access tries to read that issue. Moving from user tokens to an app token brings another issue - rate limits for API requests, with a single token they can be hit pretty quickly as the default limit is pretty low at around 1000 request if I remember correctly.

https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28 It's 5000-15000 requests per hour. We don't want to go there, we'll for sure gonna hit it

Say 6000/hour, that's 100/min, 1.5/sec - how many GH issues do we need to handle concurrently? How many do we even have in the system?

Large releases like scylla master have a lot of issues, and if someone decides to page through all of them on the release dashboard page - they might contribute to this limit.

However I admit that I think that hitting the limit is not as easy anymore - issue widget is loaded on demand and is paged (each page caching the result until a hard refresh), so this limit should be fairly hard to hit (unless of course 50 people decide to open 100 issues all in the same hour)

We've been burned by this in dtest, we estimated some type of usage.

But the feature became much more popular, and we started crossing the limit on a daily basis.

There are multiple pages on Argus that just by browsing into them it would do multiple API calls to GitHub