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

landing page: proxying and handling Github stars fetch requests #1019

Closed
iAdramelk opened this issue Feb 24, 2020 · 8 comments · Fixed by #1363
Closed

landing page: proxying and handling Github stars fetch requests #1019

iAdramelk opened this issue Feb 24, 2020 · 8 comments · Fixed by #1363
Assignees
Labels
A: website Area: website p1-important Active priorities to deal within next sprints type: enhancement Something is not clear, small updates, improvement suggestions

Comments

@iAdramelk
Copy link
Contributor

We have stars counter for the dvc repo on the title page. And it is not always works:

https://sentry.io/organizations/iterative/issues/1510361492/

Also the it not works we didn't handle rejections in GithubLine:

https://sentry.io/organizations/iterative/issues/1519509024/

Maybe we should start proxying and caching GR request on our and as we do now for community page. And also we should add catch to request.

@shcheklein shcheklein added website: eng-doc DEPRECATED JS engine for /doc A: website Area: website type: enhancement Something is not clear, small updates, improvement suggestions and removed website: eng-doc DEPRECATED JS engine for /doc labels Feb 24, 2020
@shcheklein
Copy link
Member

@iAdramelk good catch, probably we should do the same as with the community page indeed, and cache results.

@shcheklein shcheklein added the p1-important Active priorities to deal within next sprints label Feb 24, 2020
@shcheklein shcheklein changed the title Proxying and handlig github fetch requests landing page: proxying and handling Github stars fetch requests Feb 24, 2020
@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Feb 25, 2020

I can't open the sentry.io URLs but OK I get the idea. In fact I see the GH API request duplicated?

image

Maybe we should start proxying and caching GR request on our and as we do now for community page. And also we should add catch to request.

I just want to repeat my Q from https://github.com/iterative/blog/pull/122#issuecomment-589873691 here:

if the API is basically a proxy to 3rd party APIs and only used for the community page and maybe blog posts, why do we need to do HTTP requests within the same API? Why not just import a module that does the fetching/processing of the 3rd party APIs?

Ivan's answer was:

mostly due to rate limits and response times, meaning we want to cache results aggressively. mostly means we need a server with some server-side caching mechanism

But what about a simple in-app memory cache instead?

Thanks

@shcheklein
Copy link
Member

But what about a simple in-app memory cache instead?

that's exactly what we do now :)

@iAdramelk
Copy link
Contributor Author

@jorgeorpinel

But what about a simple in-app memory cache instead?

Do you mean on the client side or in the browser?

@jorgeorpinel
Copy link
Contributor

OK yes, but I mean without the need for an internal API that proxies them to the same web app: just import some module that fetches the data and caches it (server side).

@rogermparent
Copy link
Contributor

I can fix this by grabbing data from GitHub at build time. The slight inaccuracy on old builds is practically a non-issue for something like stars, and it requires no runtime requests or logic.

I actually have a branch moving most everything to this setup, but I think I can backport the bit that fixes this issue specifically.

@rogermparent rogermparent self-assigned this May 26, 2020
@shcheklein
Copy link
Member

@rogermparent I would say, let's do this the same way as we do for other APIs. It's quite important for this counter to be up to date as much as possible.

Also, if GH down we'll have to rely on some default value or freeze the deployment.

@rogermparent
Copy link
Contributor

I think I'm pretty much done, but we'll see if I missed anything.

My solution was to implement a Gatsby build-time solution primarily, but then update it via a proxy server API like the other ones. This way we get the advantages of a solid baked-in value as well as the ability to keep the count updated to-the-minute regardless of when the last rebuild was.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: website Area: website p1-important Active priorities to deal within next sprints type: enhancement Something is not clear, small updates, improvement suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants