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

Hovercard urls are case sensitive #211

Open
xt0rted opened this issue May 18, 2020 · 3 comments
Open

Hovercard urls are case sensitive #211

xt0rted opened this issue May 18, 2020 · 3 comments

Comments

@xt0rted
Copy link
Member

xt0rted commented May 18, 2020

While adding support for nuget.org I ran into a few packages where the github url associated with it is lowercase and the hovercard endpoint returns a 404 and breaks the UI.

Recently most of the aspnet/* repos were rolled into the dotnet org so if you go to their old url github redirects you (for example https://github.com/aspnet/scaffolding now goes to https://github.com/dotnet/scaffolding).

For Microsoft.VisualStudio.Web.CodeGeneration.Tools we resolve to https://github.com/aspnet/scaffolding but the original repo name was Scaffolding. The hovercard endpoint we're building is https://github.com/aspnet/scaffolding/hovercard but these endpoints are case sensitive for some reason so it doesn't resolve and redirect to https://github.com/dotnet/Scaffolding/hovercard as expected. If you change the repo name to Scaffolding it will resolve and redirect.

I'm wondering if we should add an extra check before caching the url so if it's for github we hit their api and get the correctly cased url and then use that.

@xt0rted
Copy link
Member Author

xt0rted commented May 18, 2020

If you hit the api with https://api.github.com/repos/aspnet/scaffolding it does resolve to the moved repo and gives you https://github.com/dotnet/Scaffolding for the html_url value. So we are able to get the correct value for these.

@stefanbuck
Copy link
Member

Very interesting discovery. I'm a bit hesitant to add an extra check for a few reasons:

  • The GitHub API rate limit for authenticated request is 5000 requests per hour.
  • This rate limit is definitely not enough if we want to verify all results.
  • Even if we limit this to nuget projects (which we shouldn't do IMO), we still need to gracefully handle the case if we run into an API-rate limit.
  • The response time will increase (if not cached already)
  • Another dependency to a third party provider (if not cached already)
  • The results last in the cache for just 12 hours, so worse case we have to repeat the steps twice a day.

Strictly speaking this is an inconsistency with the GitHub API and maybe it's worth raising a support ticket. Also, the nuget packages provide a wrong url in the first place and maybe we can raise awareness on their side as well.

@xt0rted
Copy link
Member Author

xt0rted commented May 19, 2020

I could add a manual check for a couple of the the dotnet/* repos. Over time this issue should go away but since the migration is still on-going it'll be a little while before everything is completely updated. I'll also look into PRing a fix for the broken packages I come across.

I could raise an issue with GitHub about the case sensitive url but since we're piggy backing on an internal feature I doubt they'll prioritize a fix for it.

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

No branches or pull requests

2 participants