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

"Remove torrent and delete data" does not work with vanilla rTorrent #654

Open
matoro opened this issue Mar 21, 2023 · 10 comments · Fixed by #655
Open

"Remove torrent and delete data" does not work with vanilla rTorrent #654

matoro opened this issue Mar 21, 2023 · 10 comments · Fixed by #655

Comments

@matoro
Copy link

matoro commented Mar 21, 2023

Hi, thank you for this great app. I use rTorrent coupled with Flood for a web interface. Everything works in Transdroid except for "Remove torrent and delete data" - it just removes the torrent, but the data remains on disk.

After researching this, it looks like Flood calls d.erase but then directly deletes the data off the filesystem itself.

Transdroid also calls d.erase but then instead sets custom5 to 1. I believe this is actually an ruTorrent-specific behavior and does not apply to vanilla rTorrent.

ruTorrent seems to call d.delete_tied I think?

What is the correct behavior here in order to make the "delete data" functionality work?

Thank you!

@bwitt
Copy link
Contributor

bwitt commented Mar 21, 2023

maybe it's d.delete_tied, since that seems to delete the underlying file?

@bwitt
Copy link
Contributor

bwitt commented Mar 21, 2023

oh but ruTorrent does send a d.set_custom5 with a 1 or 2 just before calling d.delete_tied, and then sends a d.erase just after the d.delete_tied. so maybe we need to send a d.delete_tied in addition to what we're sending now.

@erickok
Copy link
Owner

erickok commented Mar 21, 2023

It's true, there is no vanilla rTorrent command for it afaik. Since ruTorrent is so ubiquitous we decided to implement it's command at some point, but yeah that won't work with Flood. And calling d.delete_tied won't help much as that would just delete the .torrent file, not the contents (as I understand it).

Flood can delete the files directly itself as it runs on the server itself; Transdroid can't do that.

@matoro
Copy link
Author

matoro commented Mar 21, 2023

@bwitt Unfortunately #655 does not fix it. I found this in the rtorrent docs which explains better what a "tied file" is, it's if you have a watch directory that you drop torrents in, then the .torrent it "tied" to that entry, so calling d.delete_tied just cleans up that file.

This seems to be the general approach that needs to be taken: https://superuser.com/a/1580313

However I have not managed to figure out how to turn this into something that can be called via the XMLRPC API.

I've been using the stock python client to experiment with this and it works great, all you need is something like this:

import xmlrpc.client
x = xmlrpc.client.ServerProxy("https://username:password@myhostname/RPC2")
x.system.listMethods()
x.download_list()
x.d.name("SOMEHASH")

etc...

@matoro
Copy link
Author

matoro commented Mar 21, 2023

Never mind, I figured it out! Here is how we can implement this over the API:

>>> x.method.list_keys("", "event.download.erased")
['!_download_list', '~_delete_tied']
>>> x.method.set_key("", "event.download.erased", "delete_erased", "execute=rm,-rf,--,$d.base_path=")
0
>>> x.method.list_keys("", "event.download.erased")
['!_download_list', 'delete_erased', '~_delete_tied']
>>> # Now delete your torrent through Transdroid.  Data is correctly wiped from disk
>>> # Restore default behavior
>>> x.method.set_key("", "event.download.erased", "delete_erased")
0
>>> x.method.list_keys("", "event.download.erased")
['!_download_list', '~_delete_tied']

Downside is that this is a completely global setting. I am using the empty string as a target (based on the discussion in Sonarr/Sonarr@444fcf5 and rakshasa/rtorrent#227) but even when specifying the hash of a particular torrent, it seems to register the handler globally regardless. Therefore if you delete two torrents at the same time from two different clients, one with "delete data" enabled and the other without, it's possible that the data would get deleted for both. But this is kind of a stretch in terms of use cases...

Note also that it seems recent rtorrent have d.delete_tied as a built-in erase trigger anyway, so there's no point in doing so manually.

Either way, this should be reasonable to port into Transdroid. @erickok would you be willing to implement this approach?

@erickok
Copy link
Owner

erickok commented Mar 23, 2023

I am not sure if we can call x.method.set_key via RPC?

If we can this would functionally work perhaps, but it seems dangerous. At the very least this completely overrides any comment a user would have bound to that event himself. I don't know... it feels kinda hacky.

Of course you can say the same of calling custom5 which might not be the remove command Transdroid currently expects...

@matoro
Copy link
Author

matoro commented Mar 23, 2023

I am not sure if we can call x.method.set_key via RPC?

If we can this would functionally work perhaps, but it seems dangerous. At the very least this completely overrides any comment a user would have bound to that event himself. I don't know... it feels kinda hacky.

Of course you can say the same of calling custom5 which might not be the remove command Transdroid currently expects...

Yes you can, I am doing all this with the Python XMLRPC client. That's what the x variable is, it's like this:

>>> import xmlrpc.client
>>> x = xmlrpc.client.ServerProxy("https://username:password@myhostname/RPC2")
>>> x.method.list_keys("", "event.download.erased")
['!_download_list', 'delete_erased', '~_delete_tied']

Furthermore I tested a few more things, method.set_key is additive, if the key is not in the list it is added, and rTorrent executes all keys on an event trigger in alphabetical order (that is why builtin keys start with either ! to put them at the front of the list, or ~ to put them at the end of the list). You can add and delete keys on demand without affecting any other keys.

The only problem comes in if the user has a custom key tied to the event.download.erased trigger that expects the files to still be present on disk. But as long as their trigger executes first (is alphabetically before our trigger) then it should be perfectly fine, since we are calling rm -rf which does not bail out if the files are already gone.

@matoro
Copy link
Author

matoro commented Dec 8, 2023

@erickok #655 does not fix this, see my explanation above.

@erickok
Copy link
Owner

erickok commented Dec 8, 2023

Yes I was afraid of that. We'll have to try your solution.

@matoro
Copy link
Author

matoro commented Dec 11, 2023

Thanks, can you re-open the issue since it's not resolved?

@erickok erickok reopened this Dec 12, 2023
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 a pull request may close this issue.

3 participants