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

Fix #456: Missing tags when a project has more than 100 tags #458

Merged
merged 2 commits into from May 18, 2021

Conversation

WebFreak001
Copy link
Member

@WebFreak001 WebFreak001 commented Apr 7, 2020

  • adds a template URI parser for the Link header in GitHub (and also other parts) where it says it may use Hypermedia links in uritemplate.d - I will release this file as separate MIT library as well, so instead of including it here it might also be worth it just depending on it
  • adds the possibility to cache headers in the urlcache module
  • adds the readPagedList function in GitHub which traverses the list

For #456 to be properly fixed in this exact case right after deployment, either a new ETag must be sent by github (which is probably done on next release) or the db.urlcache.entries in vpmreg needs to be dropped by a DB administrator (s-ludwig) and all URLs should automatically be regrabbed. I think doing the latter will prevent some pain as cache may be cached for up to a year and I'm not exactly sure when GitHub will send new ETag values.

On localhost this fix now made mir-algorithm have 332 versions instead of 114 (100 tags + branches) again.

@dlang-bot
Copy link
Collaborator

Thanks for your pull request, @WebFreak001!

@WebFreak001 WebFreak001 changed the title Fix #456 [WIP] Fix #456 Apr 7, 2020
@dlang-bot dlang-bot added the WIP label Apr 7, 2020
@WebFreak001 WebFreak001 changed the title [WIP] Fix #456 Fix #456 Apr 7, 2020
@WebFreak001 WebFreak001 added WIP and removed WIP labels Apr 7, 2020
@schveiguy
Copy link
Member

How are we doing on this? The bug to be fixed has broken all buildkite builds because of a dependency on a really old dub version. I can create a temporary fix for dub if needed, but I'm hoping this can be merged soon instead.

See for instance https://buildkite.com/dlang/phobos/builds/3239#8a55202b-60b6-4cc8-a467-7642e8175643/109-1009

ping @s-ludwig @WebFreak001

@Geod24 Geod24 changed the title Fix #456 Fix #456: Missing tags when a project has more than 100 tags May 1, 2020
@WebFreak001
Copy link
Member Author

uhh just waiting on review, test case failure doesn't seem related to my PR because it's some botan failures

@schveiguy
Copy link
Member

schveiguy commented May 1, 2020

The test case is definitely related. Essentially, when I do dub -v upgrade in that test directory (specifically here: https://github.com/dlang/dub/tree/master/test/issue672-upgrade-optional), it lists all the dub package dependencies, and both 1.0.0 and 1.1.0 are missing. This is because they've been eclipsed off the first page of tags, and code.dlang.org has "forgotten" them.

Doh! I thought you were talking about my linked case, but you are talking about the test failure for this PR. Nevermind.

@WebFreak001
Copy link
Member Author

I think this might also fix some maybe-issues with dub registry constantly refetching old versions

@WebFreak001
Copy link
Member Author

@s-ludwig do you want me to put out the uritemplate.d in some separate package?

@wilzbach
Copy link
Member

@s-ludwig do you want me to put out the uritemplate.d in some separate package?

That would be better. If possible. Thanks!

@s-ludwig
Copy link
Member

s-ludwig commented May 7, 2021

@s-ludwig do you want me to put out the uritemplate.d in some separate package?

That would be better. If possible. Thanks!

Would definitely be nice, but I wouldn't make this a requirement for the PR. We should keep this on the radar, though.

From my side this is good to go after a rebase.

Just need to rememeber to clear the cache after this gets deployed to get all headers cached.

@WebFreak001
Copy link
Member Author

the package is now released: https://github.com/WebFreak001/URITemplate

@WebFreak001
Copy link
Member Author

@s-ludwig I have upgraded the code onto latest master, included the request_modifier (for adding authentication) and use uritemplate as dub package now.

I haven't run the unittests with MongoDB enabled though (for cache test) and haven't tried out if this works yet. I don't have a dub registry setup right now so it would be nice if you could do that for me here.

Copy link
Member

@s-ludwig s-ludwig left a comment

Choose a reason for hiding this comment

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

I've tested locally, both unit tests and running the actual registry, and all seems to work fine, including fetching projects with >100 tags. This just needs another rebase due to dub.selections.json and can be merged. I'll wait with the deployment until tomorrow though, to first see how the effect of the update queue fixes is in practice.

@WebFreak001
Copy link
Member Author

ok rebased, ready for merge

@s-ludwig s-ludwig merged commit c631a70 into dlang:master May 18, 2021
@s-ludwig
Copy link
Member

This is deployed now.

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.

None yet

5 participants