search

autodl-community / autodl-irssi

A community-driven fork of autodl-irssi
https://autodl-community.github.io/autodl-irssi/
374 stars 73 forks source link

Fix for the issue "Torrent size as hard filtering criteria #201" #202

Open satcos opened 4 years ago

satcos commented 4 years ago

This is quick workaround for the issue "Torrent size as hard filtering criteria #201" https://github.com/autodl-community/autodl-irssi/issues/201#issue-655679838

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.

thebigmunch commented 4 years ago

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 commented 4 years ago
  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 commented 4 years ago

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.