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

use single api request to fill metadata cache for all dependencies #1366

Merged
merged 5 commits into from Apr 9, 2019

Conversation

Projects
None yet
5 participants
@MartinNowak
Copy link
Member

commented Feb 9, 2018

  • avoids various RTTs and possible failures

Requires dlang/dub-registry#278

@dlang-bot

This comment has been minimized.

Copy link
Collaborator

commented Feb 9, 2018

Thanks for your pull request, @MartinNowak!

@wilzbach
Copy link
Member

left a comment

Nice! I guess we have to wait until Sönke has time to rollout the changes to the registry? We should really get auto-deploy for it ...

@s-ludwig

This comment has been minimized.

Copy link
Member

commented Feb 12, 2018

Deployment is currently blocked until vibe-d/vibe.d#2037 is merged. I have it at the top of my list of things to review.

@wilzbach wilzbach added this to the 1.9.0 milestone May 6, 2018

@wilzbach wilzbach force-pushed the MartinNowak:single_rtt_upgrade branch from d392c16 to 59cae10 Jun 25, 2018

@wilzbach

This comment has been minimized.

Copy link
Member

commented Jun 25, 2018

Rebased to master.
As the server dependency has been merged and deployed, this should be ready to go now.

@wilzbach wilzbach modified the milestones: 1.9.0, 1.10.0 Jun 25, 2018

@wilzbach wilzbach closed this Jul 17, 2018

@wilzbach wilzbach reopened this Jul 17, 2018

@wilzbach wilzbach modified the milestones: 1.10.0, 1.11 Jul 21, 2018

@wilzbach

This comment has been minimized.

Copy link
Member

commented Jul 21, 2018

dlang/dub-registry#278 is now merged, but it still needs to be deployed ...

@wilzbach wilzbach modified the milestones: 1.11, 1.12 Aug 20, 2018

@MartinNowak

This comment has been minimized.

Copy link
Member Author

commented Dec 24, 2018

I ultimately think that finishing the git index work is more fruitful. It would allow us to fully decouple dub from the registry (and only send download stats via UDP).

@wilzbach wilzbach force-pushed the MartinNowak:single_rtt_upgrade branch from 59cae10 to eba19f9 Apr 7, 2019

@wilzbach wilzbach added the auto-merge label Apr 7, 2019

@wilzbach

This comment has been minimized.

Copy link
Member

commented Apr 7, 2019

Rebased this (this should have been deployed by now).

@wilzbach

This comment has been minimized.

Copy link
Member

commented Apr 7, 2019

Argh. So apparently the new /infos endpoint wasn't exposed in the dub-registry PR:

https://github.com/dlang/dub-registry/pull/278/files#diff-208dd7dc9f2b52b6520f8975b543f926R98

(It looks like it was an oversight due to refactoring of the PR.

Anyhow, we can already use the new minimize flag and I opened a PR to do so, (#1676), s.t. we can at least this improvement in the next release.

@wilzbach

This comment has been minimized.

Copy link
Member

commented Apr 7, 2019

Argh. So apparently the new /infos endpoint wasn't exposed in the dub-registry PR:
https://github.com/dlang/dub-registry/pull/278/files#diff-208dd7dc9f2b52b6520f8975b543f926R98

PR to fix this: dlang/dub-registry#422

@wilzbach wilzbach referenced this pull request Apr 7, 2019

Closed

Expose /infos endpoint #422

@wilzbach wilzbach force-pushed the MartinNowak:single_rtt_upgrade branch from fd40ea5 to 21d62b3 Apr 8, 2019

catch(HTTPStatusException e) {
if (e.status == 404) {
logDebug("Package %s not found at %s (404): %s", packageId, description, e.msg);
return Json(null);

This comment has been minimized.

Copy link
@wilzbach

wilzbach Apr 8, 2019

Member

This was really bad, because at the moment the https://dub-registry.herokuapp.com returned 404 for the package. This lead to a null Json being returned and thus the mirror request being accepted as successful.

@wilzbach

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

Thanks to @s-ludwig's advice, I think I managed to rebase this properly and adjust the URL, s.t. we can ship this finally with 2.086.

However, I had a quick lock at the log when building vibe-d and it looks like the include_dependencies=true flag doesn't include all dependencies:

> dub --vverbose
...
Checking for missing dependencies.
Downloading metadata for vibe-core
Getting https://code.dlang.org/api/packages/infos?packages=%5B%22vibe-core%22%5D&include_dependencies=true&minimize=true...
adding vibe-core to metadata cache
adding memutils to metadata cache
adding taggedalgebraic to metadata cache
adding mir-core to metadata cache
adding stdx-allocator to metadata cache
adding eventcore to metadata cache
adding libasync to metadata cache
Downloading metadata for botan-math
Getting https://code.dlang.org/api/packages/infos?packages=%5B%22botan-math%22%5D&include_dependencies=true&minimize=true...
adding botan-math to metadata cache
Downloading metadata for botan
Getting https://code.dlang.org/api/packages/infos?packages=%5B%22botan%22%5D&include_dependencies=true&minimize=true...
adding botan to metadata cache
adding botan-math to metadata cache
adding memutils to metadata cache
adding openssl to metadata cache
Downloading metadata for diet-ng
Getting https://code.dlang.org/api/packages/infos?packages=%5B%22diet-ng%22%5D&include_dependencies=true&minimize=true...
adding diet-ng to metadata cache
Downloading metadata for mir-linux-kernel
Getting https://code.dlang.org/api/packages/infos?packages=%5B%22mir-linux-kernel%22%5D&include_dependencies=true&minimize=true...
adding mir-linux-kernel to metadata cache
Downloading metadata for libevent
Getting https://code.dlang.org/api/packages/infos?packages=%5B%22libevent%22%5D&include_dependencies=true&minimize=true...
adding libevent to metadata cache
...

Though a lot of dependencies are already loaded from the metadata cache which is a big improvement (:

@wilzbach

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

CC @thewilsonator as this is another important DUB PR that ideally we should get into 2.086.

@thewilsonator

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

Is travis expected to fail? Otherwise LGTM.

@wilzbach

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

That's weird - master passes: https://travis-ci.org/dlang/dub/branches

This change apparently makes LDC 1.13.0 segfault on 32-bit:

Generating version file...
Running /home/travis/dlang/ldc-1.13.0/bin/ldmd2...
collect2: error: ld terminated with signal 11 [Segmentation fault], core dumped

I tried to bump it to 1.15.0 and hope for the best.

@wilzbach wilzbach added the auto-merge label Apr 9, 2019

@dlang-bot dlang-bot merged commit 2aafe39 into dlang:master Apr 9, 2019

5 checks passed

buildkite/dub Build #483 passed (1 hour, 3 minutes, 55 seconds)
Details
codecov/patch 100% of diff hit (target 69.11%)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
semaphoreci The build passed on Semaphore.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.