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

MS api throttling mitigation #203

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

lukaarma
Copy link
Collaborator

As told you in the last PR here is the change that will help with #170, so now it should not request thing in bulk so that it doesn't get soft-banned for 10 sec

fixed side effects in main function of this change
@lukaarma lukaarma requested a review from snobu August 12, 2020 17:48
@lukaarma
Copy link
Collaborator Author

Oh @snobu, please update the subversion number and maybe a little bit of description? Also about the title template since I don't remember if we added that last update

@snobu
Copy link
Owner

snobu commented Aug 15, 2020

Looks like we have some conflicts now, can you resolve them before we go to review?

let i = 0;
finalTitle = title;
while (match) {
let value = video[match[1] as keyof Video] as string;
Copy link
Owner

@snobu snobu Aug 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(____/)
( ͡ ͡° ͜ ʖ ͡ ͡°)
\╭☞ \╭☞. Love the keyof T here.

@lukaarma
Copy link
Collaborator Author

Looks like we have some conflicts now, can you resolve them before we go to review?

Sure can, between this evening and tomorrow I'll push

@snobu
Copy link
Owner

snobu commented Aug 15, 2020

Looks like after this change we won't be able to verify things upfront, meaning if one of the URLs returns 404 (invalid GUID or video has been deleted by owner) we won't be able to catch it early. What if we introduce a small delay before calling the API for each GUID yet we still do it all upfront? Did you notice aggressive throttling from the MSStream API, meaning can we do more than one API call a second?

Edit:
Nevermind you already described this in #170. I would expect 429 Too Many Requests coming back, not a hard TCP RST. Let me run a few tests myself see if we're consistent with findings.

@snobu
Copy link
Owner

snobu commented Aug 15, 2020

Bizarre, i'm unable to go into throttling. Tried both HTTP/1.1 and HTTP/2, even 10 req/second work out fine. However throttling may not be even across all API endpoints, i'm only able to call https://use2-2.api.microsoftstream.com.

Not sure how to approach this right now to be honest. I'd say let's park it for now unless there are more throttling reports coming in and then maybe we have more data to draw a more meaningful conclusion.

@lukaarma
Copy link
Collaborator Author

I occur into it especially with groups link, where there are like 30+ videos
I found that after the first 10 stop another 10 stop then 5 stop then 1 at a time with a stop in between each

I tried a delay of 1 2 and 3 secs and all of them get throttled after the 10th
But it may be no longer like this, I could do some more tests

But I agree with you, let's park this and let's see how it goes

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

Successfully merging this pull request may close these issues.

2 participants