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

support S3 multipart uploads for large NARs #2139

Merged
merged 3 commits into from May 10, 2018

Conversation

AmineChikhaoui
Copy link
Member

No description provided.

if (contentEncoding != "")
request.SetContentEncoding(contentEncoding);
auto executor =
std::make_shared<Aws::Utils::Threading::PooledThreadExecutor>(maxThreads);
Copy link
Member

Choose a reason for hiding this comment

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

This thread pool should be shared between invocations, otherwise e.g. nix copy will start cores^2 threads (since it will do multiple copies in parallel). E.g.

static std::make_shared<Aws::Utils::Threading::PooledThreadExecutor> = std::make_shared<...>(maxThreads);

Copy link
Member Author

Choose a reason for hiding this comment

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

@edolstra I pushed a fix in case you missed it :)

printTalkative("upload progress ('%s'): '%d' of '%d' bytes",
path,
transferHandle->GetBytesTransferred(),
transferHandle->GetBytesTotalSize());
Copy link
Member

Choose a reason for hiding this comment

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

This looks like it could really spam the console. It should use Nix's progress API.

Copy link
Member

Choose a reason for hiding this comment

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

I guess it's fine since it requires -v.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah it's only when you specify a higher debug level, not sure how many v's though :D

@edolstra edolstra merged commit 854c086 into NixOS:master May 10, 2018
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

3 participants