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 for the issue "Torrent size as hard filtering criteria #201" #202

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

satcos
Copy link

@satcos satcos commented Jul 19, 2020

This is quick workaround for the issue "Torrent size as hard filtering criteria #201"
#201 (comment)

The fix is designed to bring minimum changes to the flow and as well as to achieve the end result
Two major changes

  1. Filter manager performs a soft check on torrent size. When "torrentSizeInBytes" is not defined, treats the condition as satisfied, if defined checks the condition.
  2. MatchedRelease when size condition is not met instead of aborting the process triggers the filter check and download process again with additional information of "torrentSizeInBytes"

Short comings of the fix

  1. Under certain conditions torrent file is downloaded twice. 2nd download of torrent file certainly leads to an action.
  2. Time and computation wasted in rechecking the filter

Possible improvements

  1. Avoid downloading torrent file again, if it is available in temp
  2. Parse torrentSizeInBytes from IRC announcement wherever possible

Please read the contributing guidelines linked above before opening a pull request.

…g criteria autodl-community#201"

autodl-community#201 (comment)

The fix is designed to bring minimum changes to the flow and as well as to achieve the end result
Two major changes
1. Filter manager performs a soft check on torrent size. When "torrentSizeInBytes" is not defined, treats the condition as satisfied, if defined checks the condition.
2. MatchedRelease when size condition is not met instead of aborting the process triggers the filter check and download process again with additional information of "torrentSizeInBytes"

Short comings of the fix
1. Under certain conditions torrent file is downloaded twice. 2nd download of torrent file certainly leads to an action.
2. Time and computation wasted in rechecking the filter

Possible improvements
1. Avoid downloading torrent file again, if it is available in temp
2. Parse torrentSizeInBytes from IRC announcement wherever possible
@thebigmunch
Copy link
Member

thebigmunch commented Oct 11, 2020

Sorry for the long delay. I've been on a prolonged hiatus from hobby coding. And thanks for the effort.

  1. Under certain conditions torrent file is downloaded twice. 2nd download of torrent file certainly leads to an action.

This is a complete non-starter. I previously talked about the changes that need to be made would be much bigger than this. This would involve using a cache to make sure this doesn't happen. The modules may even be reorganized to group the size checking with the rest of the filtering.

  1. Parse torrentSizeInBytes from IRC announcement wherever possible

Nope. This was purposely changed for consistency and simplicity. Having differing behaviors also caused confusion among enough users to be a problem.

@satcos
Copy link
Author

satcos commented Oct 13, 2020

  1. Under certain conditions torrent file is downloaded twice. 2nd download of torrent file certainly leads to an action.

By this I mean, When all conditions pass ignoring torrent size(approx match) we download the torrent file (Current scenario), That gives us additional information about torrent size. With the additional info check all filters, if there is a matching filter proceed with download and it will certainly lead to an action because we got a 100% (including torrent size) matching filter.

Hope the over-ahead caused by the fix is minimal as 1st download is a small fraction of approx match and 2nd download is small fraction of 1st download.

This fix makes the tool functionally correct.

Are we good to go ahead with the merge?

@thebigmunch
Copy link
Member

Hope the over-ahead caused by the fix is minimal as 1st download is a small fraction of approx match and 2nd download is small fraction of 1st download.

There are trackers that limit the number of torrents that can be downloaded from them in a time period. Adding more unnecessarily is not a viable option.

This fix makes the tool functionally correct.

This tool is already functionally correct.

Are we good to go ahead with the merge?

No. I've already stated the things that need to occur in a change to this to be mergeable.

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.

2 participants