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

Feature: add torrent selection for Pause & Resume commands #33

Merged

Conversation

xel86
Copy link
Contributor

@xel86 xel86 commented Feb 7, 2022

Added the same functionality from newly merged Remove command to Pause & Resume commands.

Same --hashes --names --all flags added to both Pause & Resume.

Also did a bit of refactoring of the code to search for torrents using name or hash prefixes, and made it into a method for the qbittorrent client in methods.go to be universally used on all of the commands mentioned. GetTorrentsByPrefixes accepts a string list of terms to search for, and boolean flags to check against either hashes, names, or both.

( I intend to add categories + tags into these commands with separate functions later :) )

@ludviglundgren ludviglundgren self-requested a review February 7, 2022 08:08
@ludviglundgren ludviglundgren added the enhancement New feature or request label Feb 7, 2022
Copy link
Owner

@ludviglundgren ludviglundgren left a comment

Choose a reason for hiding this comment

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

Nice! But i do see one problem tho. The pause and resume previously used the "all" flag but with the refactor it does not. There's limits on how much you can send to the api - so say you have 1000+ torrents running then that will be a lot of hashes in one go and it will probably fail.

This is true for all methods touching all.

The "all" param is a built in feature of the qbit api where it handles the ids on the backend instead of sending all the ids.

I have implemented a batch function in another repo that mitigates this. I'll share that during the day so you can implement it where necessary 🙂

@ludviglundgren
Copy link
Owner

Turns out the batch function was in this repo all along 🙈

Check this: https://github.com/ludviglundgren/qbittorrent-cli/blob/develop/cmd/compare.go#L106-L121

// Split the slice into batches of 20 items.
batch := 20
for i := 0; i < len(duplicateTorrents); i += batch {
	j := i + batch
	if j > len(duplicateTorrents) {
		j = len(duplicateTorrents)
	}

	qbCompare.SetTag(duplicateTorrents[i:j], "duplicate")

	// sleep before next request
	time.Sleep(time.Second * 1)
}

This grabs 20 items and runs the setTag in this case. And adds a sleep to be a bit nicer to qbit and not hammer it too hard. Could probably be lowered to 500ms as well but this gives some margin. It will take some time to run on 1000+ torrents but rather that than breaking things.

@xel86
Copy link
Contributor Author

xel86 commented Feb 7, 2022

I've added the batching technique to all the commands, used the "all" endpoint for pause/resume/remove when applicable (when user uses --all flag), and made the prefix search slightly faster.

I repeated the batching loop for all three commands and contemplated making some sort of batching method with a function pointer to use for the request, but I restrained myself lol. Let me know what you think!

+squashed commits

@xel86 xel86 force-pushed the feature/remove-pause-selection branch from 8caeb76 to 052ab0d Compare February 7, 2022 17:32
@ludviglundgren
Copy link
Owner

Awesome! 😄

Hehe yeah, sure that would be nice but a some copying is better than over-engineering 😄

Do you have more changes in mind in other PRs, or can I go ahead and release this?

@xel86
Copy link
Contributor Author

xel86 commented Feb 7, 2022

Awesome! smile

Hehe yeah, sure that would be nice but a some copying is better than over-engineering smile

Do you have more changes in mind in other PRs, or can I go ahead and release this?

You can go ahead and release this! 😄

@ludviglundgren
Copy link
Owner

Great! If you're interested in other tools related to qbit and torrents, my other project is autobrr.

We have a growing Discord server that you're more than welcome to join 🙂

@ludviglundgren ludviglundgren merged commit 3c4c6b4 into ludviglundgren:develop Feb 7, 2022
@Dan1jel
Copy link

Dan1jel commented Nov 7, 2023

Sorry to ask in old thread, but im trying to pause via name but it always fails, even tho its the whole name. How should this be used?

Hashes works, the whole hash, would be nice to have like a minimum of 5 (?!) letters/numbers?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants