Skip to content
This repository has been archived by the owner on Jul 4, 2024. It is now read-only.

Add --threads (-t) option to downloading commands #256

Merged
merged 4 commits into from
Oct 15, 2023
Merged

Add --threads (-t) option to downloading commands #256

merged 4 commits into from
Oct 15, 2023

Conversation

valentinegb
Copy link
Contributor

@valentinegb valentinegb commented Oct 13, 2023

This PR adds a threads field to DownloadBuilder and Downloader and adds a --threads (or -t for short) option to the download and archive commands. This overrides the cores that Downloader utilizes for downloading. This is useful in situations such as my own where having one program utilize all cores for downloading renders the rest of the device practically unusable. I understand that the name --threads might not be entirely accurate as to what it does since I think the number of threads tokio uses is the same to some extent even with this new option, so I'm open to suggestions for a more accurate name.

@bytedream
Copy link
Member

This could be extended to be a command to specify the thread count instead of only single threaded

@valentinegb
Copy link
Contributor Author

This could be extended to be a command to specify the thread count instead of only single threaded

True, that'd be pretty easy, but do you mean a command or an option? I feel like an option would make more sense.

@bytedream
Copy link
Member

bytedream commented Oct 14, 2023

Yes an option/flag. So instead of --single-threaded, --threads which takes the thread number as argument

@valentinegb
Copy link
Contributor Author

Yes an option/flag. So instead of --single-threaded, --threads which takes the thread number as argument

Okay, sure, I'll do that later today 👍

@valentinegb valentinegb changed the title Add --single-threaded (-t) option to downloading commands Add --threads (-t) option to downloading commands Oct 14, 2023
@valentinegb
Copy link
Contributor Author

Yes an option/flag. So instead of --single-threaded, --threads which takes the thread number as argument

Done :)

@bytedream
Copy link
Member

Okay nice. Just one more thing which came me in mind. Instead of the flag being Option<usize>, it could directly be usize with the default value of num_cpus::get. With this the user directly knows how many threads are used by default when doing crunchy-cli <command> --help

@valentinegb
Copy link
Contributor Author

Okay nice. Just one more thing which came me in mind. Instead of the flag being Option<usize>, it could directly be usize with the default value of num_cpus::get. With this the user directly knows how many threads are used by default when doing crunchy-cli <command> --help

You got it 👍

Copy link
Member

@bytedream bytedream left a comment

Choose a reason for hiding this comment

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

Looks good :) 👍

@bytedream bytedream merged commit bbb5a78 into crunchy-labs:master Oct 15, 2023
@valentinegb valentinegb deleted the single-threaded-downloads branch October 15, 2023 18:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants