New: Flood Download Client #4159

jesec · 2020-12-13T14:38:59Z

Database Migration

NO

Description

Add support for Flood, a modern Web UI for various torrent clients.

jesec · 2020-12-25T05:24:29Z

Happy holidays!

I would be appreciated if you can give some preliminary reviews. I deployed the PR on my Sonarr/Radarr instances, and the PR is working perfectly fine so far.

Thanks.

BTW: I joined the Sonarr Discord. Feel free to contact me directly if necessary.

markus101

Haven't tested, but left some preliminary comments.

markus101 · 2021-01-03T18:26:46Z

src/NzbDrone.Core/Download/Clients/Flood/Flood.cs

+            _proxy = proxy;
+        }
+
+        private static IEnumerable<string> HandleTags(RemoteEpisode media, FloodSettings settings)


media -> remoteEpisode

markus101 · 2021-01-03T18:32:10Z

src/NzbDrone.Core/Download/Clients/Flood/FloodSettings.cs

+        public bool StartOnAdd { get; set; }
+
+        [FieldDefinition(12, Label = "Sequential Download", Type = FieldType.Checkbox, Advanced = true)]
+        public bool SequentialDownload { get; set; }


We do not have plans to support this for other clients, so we don't want to support it for flood either.

SequentialDownload removed.

markus101 · 2021-01-03T18:40:07Z

src/NzbDrone.Core/Download/Clients/Flood/FloodSettings.cs

+            RuleFor(c => c.Host).ValidHost();
+            RuleFor(c => c.Port).InclusiveBetween(1, 65535);
+
+            RuleFor(c => c.TargetRatio).GreaterThanOrEqualTo(0).Unless(c => c.TargetRatio == -1);


Ratio limits in clients are based on indexer settings, there shouldn't be a setting for flood.

TargetRatio removed.

markus101 · 2021-01-03T18:42:52Z

src/NzbDrone.Core/Download/Clients/Flood/Flood.cs

+        {
+            var result = item.Clone();
+
+            var contentPaths = _proxy.GetTorrentContentPaths(item.DownloadId, Settings);


Can this be fetched when the torrents are fetched? Avoiding the extra API call? The output path is already being set in GetItems what is that directory if it's not where it should be imported from?

Unfortunately, no. Get torrent API call provides directory path only. There are many cases: single-file vs. multi-file, base directory vs. sub directory.

When the torrent is single-file, Sonarr expects path to the file. Otherwise, it will try to import the whole directory, which can cause problems if there are unrelated files in the directory. For multi-file torrents, Sonarr expects path to the base directory of the torrent.

As such, an additional API call to fetch the list of contents of a torrent is required. It is more efficient (resource saving) to fetch when the import is requested.

markus101 · 2021-01-03T18:51:37Z

src/NzbDrone.Core/Download/Clients/Flood/Flood.cs

+                    item.Status = DownloadItemStatus.Paused;
+                }
+
+                if (Settings.TargetRatio != -1 && item.SeedRatio > Settings.TargetRatio)


As mentioned on the setting the target ratio comes from the indexer, there is a big discussion on how to do this in rtorrent here: #3632

The big thing is we don't want Sonarr to be a seedtime/ratio manager, it can pass the limits to the client, but the client should be able to indicate when the time/ratio has been met.

TargetRatio removed.

markus101

Found a couple issues while testing, will be able to test more once the path issue is resolved.

src/NzbDrone.Core/Download/Clients/Flood/Flood.cs

markus101 · 2021-01-14T01:55:36Z

src/NzbDrone.Core/Download/Clients/Flood/Flood.cs

+            {
+                // For multi-file torrent, OutputPath should be the path of base directroy of torrent.
+                // isBasePath is false by default. Presumably base directory would be first fragment of path of content.
+                result.OutputPath = item.OutputPath + new OsPath(contentPaths[0].Split(new char[] { '\\', '/' }, StringSplitOptions.RemoveEmptyEntries)[0]);


This is not working for multi-file torrents. It currently returns the path to the first file, but instead I believe it should just be returning item.OutputPath.

item.OutputPath is the directory of TorrentProperties, which, by definition, is always a directory. This directory may or may not be the base directory. This causes problems as Sonarr can grab contents of unrelated torrents which reside in the same destination directory.

This line of code tries to combine the directory with the first segment (directory) of a content when the torrent is multi-file:

TorrentProperties.directory + first path segment of first content of a multi-file torrent (path -> split -> pathSegments[0])

/mnt/data0/downloads + Big.Buck.Bunny.1080P/~~Soundtracks/Bunny.wav~~

I tested with a multi-file torrent. It seems that this approach works fine as torrent clients always create a sub-directory for multi-file torrents by default.

For single-file torrents, Sonarr expects path to a single file, which is handled by another case (above).

What's not working? Did this line of code fail to get the first segment, combine paths, or...?

It currently returns the path to the first file.

It gave me the path to a text file, not a folder. Even if it happened to give me a video file it'd be wrong for a multi-video file download.

result.OutputPath is set to C:\downloads\torrents\test\The.Rookie.S03E02.WEBRip.x264-ION10\RARBG.txt.
item.OutputPath is set to C:\timebomb\downloads\torrents\test\The.Rookie.S03E02.WEBRip.x264-ION10

For multi-file this should just return the path, but if there is single file torrent inside a job folder you're probably going to see issues.

In your case, directory is the base directory, which is indeed a possible case. This case is not unhandled here. I assume there are some differences in how clients handle root directory of a multi-file torrent. I will work out a more robust method for detection of base directory.

Yup, they pretty much universally suck at it, we're sometimes left guessing which has lead to issues in the past and somewhat recently with qbit.

It is done. Please try it.

BTW: Sonarr can probably utilize a method that returns an array of paths (of contents of torrents) instead. That allows things to be much more precise and eliminates any interferences and inconsistencies.

Working much better now. In the future please don't squash fixes immediately, we can do that when merging the PR.

Ack. It is a shame that Github PR does not have the capability to compare different versions of change sets directly. I typically use more professional Code Review tools like Gerrit, which does have the capability (and as result, pushing additional commits can be "stupid").

Fortunately, though, there is a commit compare function: https://github.com/jesec/Sonarr/compare/17237a0e..0cae35f3.

markus101

Working great, just the warning to add then we'll get it landed.

markus101 · 2021-01-16T23:33:54Z

src/NzbDrone.Core/Download/Clients/Flood/Flood.cs

+        }
+
+        public override string Name => "Flood";
+


Until we have remove support we need to add:

public override ProviderMessage Message => new ProviderMessage("Sonarr is unable to remove torrents that have finished seeding when using Flood", ProviderMessageType.Warning);

Actually the current behavior is that Sonarr defaults to instant removal of completed torrents, which is not a desired behavior in BitTorrent community.

If we want to add the warning, I think it would be a good idea to make item.CanMoveFiles and item.CanBeRemoved always false so Sonarr won't instant remove completed torrents. Otherwise the warning can be misleading and needs to be reworded.

Yeah you’re absolutely correct, I missed that part. That + the warning then, please.

jesec · 2021-01-20T01:05:28Z

@markus101 Can you check what happened with CI? I don't have access to TeamCity.

bakerboy448 · 2021-01-20T01:12:16Z

@jesec there’s the little login as guest button on the page! (Not obvious, I missed it too!)

jesec · 2021-01-20T01:16:35Z

@jesec there’s the little login as guest button on the page! (Not obvious, I missed it too!)

Thanks.

I think it is a flaky test which does not appear to be related to this PR.

markus101 · 2021-01-21T07:44:14Z

Thanks for all the work on this @jesec!

fryfrog mentioned this pull request Jan 2, 2021

SSL Certificate Validation for Localhost #4211

Closed

markus101 requested changes Jan 3, 2021

View reviewed changes

jesec requested a review from markus101 January 4, 2021 00:52

markus101 requested changes Jan 14, 2021

View reviewed changes

jesec requested a review from markus101 January 14, 2021 04:07

markus101 requested changes Jan 16, 2021

View reviewed changes

jesec requested a review from markus101 January 17, 2021 02:17

Initial support for Flood

a8ca035

markus101 changed the title ~~Initial support for Flood~~ New: Flood Download Client Jan 21, 2021

markus101 merged commit 28e0ad3 into Sonarr:phantom-develop Jan 21, 2021

jesec mentioned this pull request Feb 2, 2021

sonarr connection to rtorrent directly jesec/rtorrent#4

Closed

This was referenced Mar 21, 2021

Add Support for Flood Lidarr/Lidarr#2104

Merged

Add Support for Flood Readarr/Readarr#917

Merged

github-actions bot locked as resolved and limited conversation to collaborators Jan 22, 2022

Uh oh!

New: Flood Download Client #4159

New: Flood Download Client #4159

Uh oh!

Conversation

jesec commented Dec 13, 2020

Database Migration

Description

Uh oh!

jesec commented Dec 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markus101 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markus101 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markus101 Jan 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jesec Jan 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markus101 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jesec commented Jan 20, 2021

Uh oh!

bakerboy448 commented Jan 20, 2021

Uh oh!

jesec commented Jan 20, 2021

Uh oh!

markus101 commented Jan 21, 2021

Uh oh!

Uh oh!

jesec commented Dec 25, 2020 •

edited

Loading

markus101 Jan 14, 2021 •

edited

Loading

jesec Jan 14, 2021 •

edited

Loading