Initial support for Flood #5552

jesec · 2020-12-13T16:58:53Z

Database Migration

NO

Description

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

Closes #5793.

austinwbest · 2020-12-13T17:18:27Z

So I guess I am confused. If it is essentially a wrapper UI around existing download clients & isn't a client, what is it you are trying to add?

jesec · 2020-12-13T17:33:11Z

So I guess I am confused. If it is essentially a wrapper UI around existing download clients & isn't a client, what is it you are trying to add?

Native support can deal with a lot of problems.

An example use case is that rTorrent XMLRPC interface is extremely insecure as it has the capabilities to execute any command. Flood, on the other side, has a modern, controlled, tested and audited API that avoids the security pitfalls.

Another use case is that the tags/categories implementation is different across clients. Flood only uses tags, and Radarr users are complaining that they can't see qBittorrent category Radarr added. jesec/flood#81

Additionally, one of the goal of the Flood project is to provide a common set of APIs across torrent clients. In the future, Flood will support more torrent clients, and Radarr may not currently support those clients. Native support also makes it easier for users to configure Radarr and Flood.

Radarr is an awesome project. I want to contribute to it and I believe this addition is useful.

austinwbest · 2020-12-13T18:28:41Z

An example use case is that rTorrent XMLRPC interface is extremely insecure as it has the capabilities to execute any command. Flood, on the other side, has a modern, controlled, tested and audited API that avoids the security pitfalls.

Ok, so you are treating flood as the client, using its API and then it will communicate with the various clients? Middle man such as Jackett for download clients i guess :)

Another use case is that the tags/categories implementation is different across clients. Flood only uses tags, and Radarr users are complaining that they can't see qBittorrent category Radarr added. jesec/flood#81

Categories work fine with Radarr/QBT. Both setting the category on grab and updating it to what you want post-import so i am not real sure what that is about? Tags are informational (which tracker it came from, etc) in QBT so they are useless when it comes to handling of the files, etc. This sounds more like a Flood issue than something that would be fixed with this PR (or i am not following what you mean)

jesec · 2020-12-13T18:46:38Z

An example use case is that rTorrent XMLRPC interface is extremely insecure as it has the capabilities to execute any command. Flood, on the other side, has a modern, controlled, tested and audited API that avoids the security pitfalls.

Ok, so you are treating flood as the client, using its API and then it will communicate with the various clients? Middle man such as Jackett for download clients i guess :)

That is correct.

Another use case is that the tags/categories implementation is different across clients. Flood only uses tags, and Radarr users are complaining that they can't see qBittorrent category Radarr added. jesec/flood#81

Categories work fine with Radarr/QBT. Both setting the category on grab and updating it to what you want post-import so i am not real sure what that is about? Tags are informational (which tracker it came from, etc) in QBT so they are useless when it comes to handling of the files, etc. This sounds more like a Flood issue than something that would be fixed with this PR (or i am not following what you mean)

Tags are defined by users. Extra information (and the ability to filter torrents based on those information) can be very useful to users who have a large collection of torrents.

Additionally, tags can be more than just informational. In Flood, tags have a role in automatically deciding the download destination when destination is not provided.

With this PR, users can just use Flood's tagging system which ensures consistency across the board and fits their needs (without having to suffer the inconsistencies of qBittorrent's categories which Flood has no plan to support, plus qBittorrent is weighing to get rid of categories in favor of tags in PR 13773).

jesec · 2020-12-27T06:38:29Z

Ping @Qstick .

I would be appreciated if you can review this pull request. I deployed the PR on my instance, and it is working perfectly fine so far.

Thanks.

Qstick · 2021-01-11T01:35:09Z

@jesec is this already updated with the changes requested that you did for Sonarr?

jesec · 2021-01-11T04:15:32Z

@jesec is this already updated with the changes requested that you did for Sonarr?

@Qstick

Not yet. I kinda prefer those features to stay if possible. "Sequential download" could prove to be more useful for larger (and non-time-sensitive) downloads like movies, which Radarr targets. I wrote about the benefits: jesec/flood#131. Meanwhile, "Target Ratio" is just an extra "if" condition, nothing else.

I would like to hear opinions of Radarr developers. Of course, I can also remove them if you would like to stick with Sonarr.

jesec · 2021-01-21T12:40:30Z

@Qstick The PR has been updated and rebased.

Qstick

Build failing due to commented variable.

In terms of Sequential and Ratio, I'd prefer to keep options tight to Sonarr.. It's a goal to eventually move extremely common areas (clients, notifications, etc) to be shared between the 4 programs... thus the less we have to deconflict them the better.

Qstick · 2021-01-25T03:49:49Z

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

+ switch (additionalTag)
+ {
+ case (int)AdditionalTags.Collection:
+ result.Add(media.Movie.Collection.Name);


media doesn't exist, thus the build failures. I think you want remoteMovie?

Qstick · 2021-01-30T20:54:52Z

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

+ [FieldDefinition(9, Label = "Additional Tags", Type = FieldType.Select, SelectOptions = typeof(AdditionalTags), HelpText = "Adds properties of media as tags. Hints are examples.", Advanced = true)]
+ public IEnumerable<int> AdditionalTags { get; set; }
+
+ [FieldDefinition(10, Label = "Start on Add", Type = FieldType.Checkbox)]


All the other clients use a setting "Add Paused" defaulted to false (same basic functionality), can we switch this around to be consistent?

Flood's API uses start. Additionally, this is already in Sonarr. It is probably better to keep it as it is.

The Flood API doesn't need to change, logic in the proxy can handle that.

I assume Sonarr just missed it on review, but I'm sure they would take a PR to fix it up for consistency sake. I don't think it's a good idea to have it different than every other client in Radarr/Sonarr from a user perspective.

It would require a database migration now to change this one.

Otherwise users will notice their settings to be unexpectedly flipped to the opposite site. I wouldn’t prefer that.

Plus, It is my belief that a good software design should use definitively affirmative (start on add) rather than unnecessary negative (add paused).

The migration on Sonarr will be a call for @markus101, but I'm sure he'd agree here on the call for consistency and be open to a quick migration for this if he deems it necessary. Regardless I'd like to see this changed for merge into Radarr.

I'm not asking you to change the default behavior, your 'StartOnAdd' defaulted to 'true' will turn into 'AddPaused' defaulted to 'false'. In either case they perform the same in the default setup. This is the expected format and behavior that both user bases currently expect and changing one client to be completely opposite adds unnecessary confusion.

It is implemented in 9b5672c. Note that I totally disagree with the change. Existing client implementation shows that the option is oriented to the APIs of respective clients. For instance, qBittorrent has InitialState instead while rTorrent/ruTorrent has nothing. All existing clients that uses AddPaused has a corresponding API interface that uses paused as well. This change makes the implementation contradictory to Flood's API.

Thanks, rTorrent has AddStopped, and yes Deluge/NzbGet/Vuze/Transmission all use AddPaused..

To your point I wouldn't be opposed centralizing them all to a InitialState field sometime in the future, as it seems a bit cleaner. If you want to go in with that on Flood I'm fine going that route, otherwise I'll merge.

You may proceed. There is no such plan at the moment. Flood's isCompleted covers such case well, and it is much more concise than ForceStart initial state from API point of view. My disappointment with the divergence from Flood's APIs stands.

Nosirus · 2021-03-08T20:26:43Z

Will there be the possibility to delete the torrent when the ratio is reached?

bakerboy448 · 2021-03-08T20:52:32Z

@Nosirus this is already in radarr (as indicated on the merged status)

but no

Radarr is unable to remove torrents that have finished seeding when using Flood

Questions-> Reddit or Discord

Qstick requested changes Jan 25, 2021

View reviewed changes

Qstick mentioned this pull request Jan 25, 2021

Pull Sonarr commit 'New: Flood Download Client' #5794

Closed

Qstick added the Status: Waiting for OP Action Required from OP label Jan 25, 2021

Initial support for Flood

398c1ad

jesec requested a review from Qstick January 25, 2021 08:33

Qstick removed the Status: Waiting for OP Action Required from OP label Jan 25, 2021

Qstick reviewed Jan 30, 2021

View reviewed changes

jesec requested a review from Qstick January 31, 2021 00:01

Flip StartOnAdd to AddPaused

9b5672c

Qstick merged commit 2237624 into Radarr:develop Jan 31, 2021

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 Mar 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial support for Flood #5552

Initial support for Flood #5552

jesec commented Dec 13, 2020 •

edited by Qstick

Loading

austinwbest commented Dec 13, 2020

jesec commented Dec 13, 2020 •

edited

Loading

austinwbest commented Dec 13, 2020

jesec commented Dec 13, 2020

jesec commented Dec 27, 2020

Qstick commented Jan 11, 2021

jesec commented Jan 11, 2021 •

edited

Loading

jesec commented Jan 21, 2021

Qstick left a comment

Qstick Jan 25, 2021

Qstick Jan 30, 2021

jesec Jan 31, 2021

Qstick Jan 31, 2021

jesec Jan 31, 2021

jesec Jan 31, 2021 •

edited

Loading

Qstick Jan 31, 2021

jesec Jan 31, 2021

Qstick Jan 31, 2021

jesec Jan 31, 2021

Nosirus commented Mar 8, 2021

bakerboy448 commented Mar 8, 2021 •

edited

Loading

Initial support for Flood #5552

Initial support for Flood #5552

Conversation

jesec commented Dec 13, 2020 • edited by Qstick Loading

Database Migration

Description

austinwbest commented Dec 13, 2020

jesec commented Dec 13, 2020 • edited Loading

austinwbest commented Dec 13, 2020

jesec commented Dec 13, 2020

jesec commented Dec 27, 2020

Qstick commented Jan 11, 2021

jesec commented Jan 11, 2021 • edited Loading

jesec commented Jan 21, 2021

Qstick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jesec Jan 31, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nosirus commented Mar 8, 2021

bakerboy448 commented Mar 8, 2021 • edited Loading

jesec commented Dec 13, 2020 •

edited by Qstick

Loading

jesec commented Dec 13, 2020 •

edited

Loading

jesec commented Jan 11, 2021 •

edited

Loading

jesec Jan 31, 2021 •

edited

Loading

bakerboy448 commented Mar 8, 2021 •

edited

Loading