Opened 14 years ago

Closed 10 years ago

Last modified 9 years ago

#1126 closed bug (Fixed)

[Extractor] Extracting not working with Move Complete enabled

Reported by: lenwar Owned by:
Priority: minor Milestone: 1.3.8
Component: Plugin Version: 1.2.0
Keywords: Extractor Cc: gareth@…

Description

I noticed that extracting does not always work properly. This happens when the downloaded (for example) .rar files have spaces in them. The files to be extracted should get " (or get escaped of course) along with them.

So:

unrar -x "totally legal thing I downloaded.rar"
or
unrar -x totally\ legal\ thing\ I\ downloaded.rar
in stead of
unrar x totally legal thing I downloaded.rar

Also note that I have my "Extracted directory" in my "completed directory"

So my relevant tree looks like so:

deluge/downloading
deluge/completed
deluge/completed/extracted

(maybe that's got something to do with it? Haven't looked at the code, so I wouldn't know)

The unrar command under Linux doesn't like it that way. (it'll only see the "totally" part of the file) I'm using 1.2.0 beta5, but earlier beta's had the same thing.

Attachments (1)

extractor-fix-and-moved-event.patch (11.6 KB) - added by Lattyware 12 years ago.
[PATCH] Fixed extractor plugin to work with move on complete - also provides a general solution for working with move on complete torrents using the TorrentMoveEvent? event.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 13 years ago by Cas

  • Component changed from other to plugin
  • Milestone changed from Future to 1.3.x

This should be checked against latest version of Deluge

comment:2 Changed 13 years ago by Cas

  • Milestone changed from 1.3.x to 1.3.2
  • Owner set to Cas
  • Status changed from new to assigned

comment:3 Changed 13 years ago by Cas

  • Milestone changed from 1.3.2 to 1.3.x
  • Owner Cas deleted

Changed 12 years ago by Lattyware

[PATCH] Fixed extractor plugin to work with move on complete - also provides a general solution for working with move on complete torrents using the TorrentMoveEvent? event.

comment:4 Changed 12 years ago by Lattyware

  • Cc gareth@… added

I have attached my patch for this issue. It provides a TorrentMoveEvent? event which can be used by plugins to manage files after they have finished being moved, rather than simply after finishing. This allows the correct handling of managing files where 'move completed' has been set.

An alternative solution to this would be to wait until after the 'move completed' action to fire the TorrentFinishedEvent? event, but I presumed that this method is more flexible, and therefore preferable.

comment:5 Changed 12 years ago by Cas

Unfortunately by adding an event to the core the patch cannot be used in 1.3-stable because it would break backward compatibility.

So that this could be included in 1.3 I think it can be achieved without core changes by simply checking if the torrent has move completed enabled then upon a finished event verify move_completed is False.

One outcome I can think of where a move complete event would be useful is if the moving storage takes a long time so you could wait for this event after the finished event. However I feel that an extra event is unneeded as a loop in plugin code watching move_completed could achieve similar functionality.

Also I am not entirely sure about the new extract option and why it is limited to only those torrent with move completed set. I have outlined the options that I think should really be covered in this plugin, is the second one the same as you had envisaged?

  • Absolute path
  • Relative path to storage location with specified folder
  • Relative path to storage location with torrent name as folder
  • Extract into torrents own folder (if no folder use above)

comment:6 Changed 12 years ago by Lattyware

My concern with simply a looping check in a plugin is mainly that of it feeling like a hack-ish solution. With large files it could mean that a plugin is sat looping for some time. Not to mention it doesn't cover the potential use case of a plugin wanting to know when a torrent is moved, not simply as a result of the move completed action.

As to the options, it was largely just I didn't think about it too much - the options you have given seem like a good coverage.

So it largely becomes a question of what is the most viable option. I would say the best option would be to simply wait until a new version to apply this so the new event could be added, but naturally, I'm don't have a good knowledge of how the project works - so this might be against the normal workflow.

comment:7 Changed 12 years ago by Cas

There is no issue adding the event to git master, which is the development branch, I was simply hoping that it could be fixed for the next 1.3 release.

If a simple hackish solution could be substituted for the event at least this would partially fix the current issue for the plugin in 1.3.

Is there logic in your patch to only extract torrents which have fired a finish event followed by a move completed?

comment:8 Changed 12 years ago by Lattyware

I see, in which case, I'll write up a simple fix for the extractor when I get the chance.

As to only extracting finished torrents, yes. The plugin has a set of torrentids, which gets added to when one finishes with move completed set, and then removed when the TorrentMovedEvent? is fired and the extraction is done.

comment:9 Changed 12 years ago by Cas

  • Summary changed from Extracting not always working to [Extractor] Extracting not working with Move Complete enabled

comment:10 Changed 11 years ago by Cas

  • Milestone changed from 1.3.x to 1.4.0

For 1.3.x I decided to disable extracting for torrents with move completed enabled.

gazpachoking:

In master if we just wait until move storage is done before issuing the torrent finished event we shouldn't have to do anything special in extractor

comment:11 Changed 10 years ago by Cas

  • Resolution set to Fixed
  • Status changed from assigned to closed

Fixed in develop: [7b44980912]

comment:12 Changed 9 years ago by SasDoe

Has this been fixed? In what deluge version? Sorry to dig this out.

comment:13 Changed 9 years ago by Cas

  • Milestone changed from 2.0.x to 2.0

It is fixed as per the ticket milestone so future major release. It's not really feasible to backport to 1.3.

comment:14 Changed 9 years ago by Cas

  • Milestone changed from 2.0 to 1.3.8

I made a mistake this was actually backported and fixed in 1.3-stable: [fed52215035b]

Note: See TracTickets for help on using tickets.