[add] deluge - Rename top-level torrent folder by andocromn · Pull Request #1790 · Flexget/Flexget

andocromn · 2017-04-13T17:31:55Z

Motivation for changes:

Currently there is no way of doing this. content_filename will rename the folder only if a main file can be identified. This does not work well with Season Packs which support has recently been added for.

Detailed changes:

Adds the option container_directory to the Deluge plugin (the container_directory verbiage was adopted from a deluge_mod.py shared to me by tubedogg)
container_directory is a jinja2 template used by client.core.rename_folder to rename the top level folder - which is effectively the torrent name in deluge
added 'name' to status_keys array to support this
rename_folder function has been added after code for content_filename to prevent interference

Config usage if relevant (new plugin or updated schema):

tasks:
  season-packs:
    rss: http://www.****.com/followed.rss
    deluge:
      label: seasonpacks
      addpaused: yes
      path: /media/tv/{{series_name}}/
      movedone: /media/tv/{{series_name}}/
      container_directory: '{{series_name}} - Season {{series_season}}'
    all_series:
      target: 720p
      timeframe: 72 hours
      upgrade: yes
      season_packs: only

To Do:

Verify conflicts do not occur when container_directory is used in conjunction with content_filename

rebase

Adds the option container_directory to the Deluge plugin container_directory is a jinja2 template used by client.core.rename_folder to rename the top level folder - which is effectively the torrent name in deluge added 'name' to status_keys array to support this

liiight · 2017-04-13T17:46:33Z

flexget/plugins/clients/deluge.py

                            opts.get('main_file_ratio') * 100))

+                if config.get('container_directory'):
+                    log.info('Renaming Folder %s to %s' % (status['name'], pathscrub(entry.render(config.get('container_directory')))))


Let the logger do its own string interpolation, pass it the args directly

removed the pathscrub. better?

Why not just render it once into a variable and then pass that variable to both the logger and the Deluge function? Saves a bit of processing.

liiight · 2017-04-13T17:47:00Z

flexget/plugins/clients/deluge.py

        config.setdefault('magnetization_timeout', 0)
        config.setdefault('keep_subs', True)  # does nothing without 'content_filename' or 'main_file_only' enabled
        config.setdefault('hide_sparse_files', False)  # does nothing without 'main_file_only' enabled
+        config.setdefault('container_directory', '')


Setting an empty default seems redundant to me

agreed, line removed

tubedogg · 2017-04-13T19:18:03Z

I haven't worked with Deluge's rename_folder before, but I wonder if it's smart enough to know if the something passed to it is not actually a folder, given that you can rename a complete folder structure by calling rename_file. If not, and the user sets container_directory with a single-file torrent that has no directory structure, it's going to end up renaming the file itself, since that's the torrent's name.

I don't think I'd rely on torrent name in either case. I'd split the (big?) file by os.sep and then call rename_folder on whichever portions you want, in your case just the top one I guess.

(I'd suggest revising the PR's name to "[add] deluge - Rename top-level torrent folder" so the changelog can pick it up easily. :))

added code to check that there is more than 1 file and abort if the torrent is a single file

liiight · 2017-04-13T20:23:26Z

flexget/plugins/clients/deluge.py


+                if config.get('container_directory'):
+                    if len(status['files']) > 1:
+                        log.info('Renaming Folder %s to %s', status['name'], entry.render(config.get('container_directory')))


I think info is a bit much for this. Maybe verbose?

liiight · 2017-04-13T20:24:42Z

flexget/plugins/clients/deluge.py

+                        log.info('Renaming Folder %s to %s', status['name'], entry.render(config.get('container_directory')))
+                        main_file_dlist.append(client.core.rename_folder(torrent_id, status['name'], pathscrub(entry.render(config.get('container_directory')))))
+                    else:
+                        log.info('container_directory specified however the number of files is not greater than 1 - skipping rename')


same here, info is too much, maybe even debug for this one

changed verification from file count greater than 1 to check for a folder structure changed to folder_rename to use name extracted from folder structure instead of torrent name removed 'name' from status_keys - thanks tubedogg for the suggestions and assistance

tubedogg · 2017-04-13T20:59:10Z

flexget/plugins/clients/deluge.py

-                        log.info('Renaming Folder %s to %s', status['name'], entry.render(config.get('container_directory')))
-                        main_file_dlist.append(client.core.rename_folder(torrent_id, status['name'], pathscrub(entry.render(config.get('container_directory')))))
+                    if opts.get('content_filename') or opts.get('main_file_only'):
+                        folder_structure = big_file_name.split(os.sep)


This will throw the error local variable 'big_file_name' referenced before assignment if content_filename or main_file_only are set but there isn't one file above the "big" (main) file threshold, or if content_filename is not set (line 626). I'd add big_file_name = '' above line 562 to ensure it's always assigned to something, and then just do if big_file_name: here.

changed logging to verbose / debug per liiight's suggestion

removed condition on main_file_only because big_file_name is only generated if content_filename is valid

changed container_directory to variable retrieved from config or set entry - container_directory option did not work if configured by set

per tubedogg's recommendation

andocromn · 2017-04-14T20:07:08Z

I've tested as many scenarios as i can think of. I tested to make sure it doesn't break anything if used in conjunction with content_filename and main_file_only. It's a bit kludgy because it ends up replacing the top level folder in content_filename with the container_directory, but it works. Probably would put a note in the wiki recommending against using both, I can't really think of a real world scenario where both would be used

I also also tested magnet links, there was an issue because the magnet has no files. I added code to check that the array 'files' is populated and cancel the rename_folder if there are no files. container_directory can be used with magnet links if convert_magnet: yes is used

liiight · 2017-04-18T18:20:11Z

You guys tested the shit out of this right? Is it ready to be merged?

tubedogg · 2017-04-18T18:23:47Z

Yeah it should be good to go.

andocromn · 2017-04-18T20:10:07Z

Yes, I did a lot of testing. there are some notes in my last comment about kludgy behaviour when using both container_directory and content_filename, it doesn't cause a problem or any errors. I could change the way this is handled, but I don't think anyone would need to use both. Just let me know if you'd like any further updates

andocromn added 2 commits April 12, 2017 14:32

Merge pull request #1 from Flexget/develop

48d6bfe

rebase

liiight reviewed Apr 13, 2017

View reviewed changes

andocromn added 2 commits April 13, 2017 14:06

updated per liiight's recommendation

a0734a0

corrected logging

409c2e2

andocromn changed the title ~~New Feature - Deluge plugin / Rename torrent (top level folder)~~ [add] deluge - Rename top-level torrent folder Apr 13, 2017

Added single file check

197a4ed

added code to check that there is more than 1 file and abort if the torrent is a single file

liiight reviewed Apr 13, 2017

View reviewed changes

tubedogg reviewed Apr 13, 2017

View reviewed changes

andocromn added 5 commits April 13, 2017 17:02

change log level

a021166

changed logging to verbose / debug per liiight's suggestion

added check for 0 files

5ab8863

fix

c510f02

removed condition on main_file_only because big_file_name is only generated if content_filename is valid

change how container_directory is retrieved

0517b35

changed container_directory to variable retrieved from config or set entry - container_directory option did not work if configured by set

changed if to big_file_name

accb43e

per tubedogg's recommendation

liiight merged commit 267ffd4 into Flexget:develop Apr 18, 2017

Conversation

andocromn commented Apr 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation for changes:

Detailed changes:

Config usage if relevant (new plugin or updated schema):

To Do:

Uh oh!

liiight Apr 13, 2017

Choose a reason for hiding this comment

Uh oh!

andocromn Apr 13, 2017

Choose a reason for hiding this comment

Uh oh!

tubedogg Apr 13, 2017

Choose a reason for hiding this comment

Uh oh!

liiight Apr 13, 2017

Choose a reason for hiding this comment

Uh oh!

andocromn Apr 13, 2017

Choose a reason for hiding this comment

Uh oh!

tubedogg commented Apr 13, 2017

Uh oh!

liiight Apr 13, 2017

Choose a reason for hiding this comment

Uh oh!

liiight Apr 13, 2017

Choose a reason for hiding this comment

Uh oh!

tubedogg Apr 13, 2017 • 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!

andocromn commented Apr 14, 2017

Uh oh!

liiight commented Apr 18, 2017

Uh oh!

tubedogg commented Apr 18, 2017

Uh oh!

andocromn commented Apr 18, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

andocromn commented Apr 13, 2017 •

edited

Loading

tubedogg Apr 13, 2017 •

edited

Loading