Skip to content

[fix] series - Added hook to add season to accepted season on any entry acceptance#2851

Merged
liiight merged 5 commits intodevelopfrom
fix_accepted_season_pack
Feb 27, 2021
Merged

[fix] series - Added hook to add season to accepted season on any entry acceptance#2851
liiight merged 5 commits intodevelopfrom
fix_accepted_season_pack

Conversation

@liiight
Copy link
Copy Markdown
Member

@liiight liiight commented Feb 11, 2021

Motivation for changes:

In case a season pack was not using the main code path, its number was not added to the exclusion list and other episode from the same task would be accepted as well.

Detailed changes:

  • Add an acceptance hook to all entries early in the filtering phase so any accepted entry which is a season pack will go through the logic of adding its number to exclusion list.

Addressed issues:

@liiight liiight requested a review from gazpachoking February 11, 2021 20:30
@soloam
Copy link
Copy Markdown
Contributor

soloam commented Feb 11, 2021

Thank you for the PR! I move my tests to this post, it's more relevant!

I tried the push request and got an error

2021-02-11 22:54:48 CRITICAL task          xTV_Test_season_pack_2 BUG: Unhandled error in plugin series: _exclude_season_on_accept() got multiple values for argument 'series_entity'
Traceback (most recent call last):

  File "/usr/lib/python3.7/threading.py", line 885, in _bootstrap
    self._bootstrap_inner()
    |    -> <function Thread._bootstrap_inner at 0x7fea69b0ce18>
    -> <Thread(task_queue, started daemon 140644677314304)>
  File "/usr/lib/python3.7/threading.py", line 917, in _bootstrap_inner
    self.run()
    |    -> <function Thread.run at 0x7fea69b0cbf8>
    -> <Thread(task_queue, started daemon 140644677314304)>
  File "/usr/lib/python3.7/threading.py", line 865, in run
    self._target(*self._args, **self._kwargs)
    |    |        |    |        |    -> {}
    |    |        |    |        -> <Thread(task_queue, started daemon 140644677314304)>
    |    |        |    -> ()
    |    |        -> <Thread(task_queue, started daemon 140644677314304)>
    |    -> <bound method TaskQueue.run of <flexget.task_queue.TaskQueue object at 0x7fea64965080>>
    -> <Thread(task_queue, started daemon 140644677314304)>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/task_queue.py", line 46, in run
    self.current_task.execute()
    |    |            -> <function Task.execute at 0x7fea67d610d0>
    |    -> <flexget.task.Task object at 0x7fea64795a20>
    -> <flexget.task_queue.TaskQueue object at 0x7fea64965080>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/task.py", line 87, in wrapper
    return func(self, *args, **kw)
           |    |      |       -> {}
           |    |      -> ()
           |    -> <flexget.task.Task object at 0x7fea64795a20>
           -> <function Task.execute at 0x7fea67d61048>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/task.py", line 722, in execute
    self._execute()
    |    -> <function Task._execute at 0x7fea67d60f28>
    -> <flexget.task.Task object at 0x7fea64795a20>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/task.py", line 688, in _execute
    self.__run_task_phase(phase)
    |                     -> 'filter'
    -> <flexget.task.Task object at 0x7fea64795a20>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/task.py", line 514, in __run_task_phase
    response = self.__run_plugin(plugin, phase, args)
               |                 |       |      -> (<flexget.task.Task object at 0x7fea64795a20>, [{'Lost (2004)': {'identified_by': 'ep', 'quality': '720p|1080p webrip+', 'tim...
               |                 |       -> 'filter'
               |                 -> <PluginInfo(name=series)>
               -> <flexget.task.Task object at 0x7fea64795a20>
> File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/task.py", line 547, in __run_plugin
    result = method(*args, **kwargs)
             |       |       -> {}
             |       -> (<flexget.task.Task object at 0x7fea64795a20>, [{'Lost (2004)': {'identified_by': 'ep', 'quality': '720p|1080p webrip+', 'tim...
             -> <Event(name=plugin.series.filter,func=on_task_filter,priority=128)>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/event.py", line 20, in __call__
    return self.func(*args, **kwargs)
           |    |     |       -> {}
           |    |     -> (<flexget.task.Task object at 0x7fea64795a20>, [{'Lost (2004)': {'identified_by': 'ep', 'quality': '720p|1080p webrip+', 'tim...
           |    -> <bound method FilterSeries.on_task_filter of <flexget.components.series.series.FilterSeries object at 0x7fea64a541d0>>
           -> <Event(name=plugin.series.filter,func=on_task_filter,priority=128)>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/components/series/series.py", line 562, in on_task_filter
    self.process_series(task, series_entries, series_config)
    |    |              |     |               -> {'identified_by': 'ep', 'quality': '720p|1080p webrip+', 'timeframe': '4 hours', 'target': '720p webrip+ h264+', 'season_pack...
    |    |              |     -> <unprintable dict object>
    |    |              -> <flexget.task.Task object at 0x7fea64795a20>
    |    -> <function FilterSeries.process_series at 0x7fea64cb8a60>
    -> <flexget.components.series.series.FilterSeries object at 0x7fea64a541d0>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/components/series/series.py", line 782, in process_series
    if self.process_timeframe_target(config, entries, downloaded):
       |    |                        |       |        -> []
       |    |                        |       -> [<Entry(title=Lost S01 720p hdtv h264,state=accepted)>]
       |    |                        -> {'identified_by': 'ep', 'quality': '720p|1080p webrip+', 'timeframe': '4 hours', 'target': '720p webrip+ h264+', 'season_pack...
       |    -> <function FilterSeries.process_timeframe_target at 0x7fea64cb8b70>
       -> <flexget.components.series.series.FilterSeries object at 0x7fea64a541d0>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/components/series/series.py", line 876, in process_timeframe_target
    entry.accept('target quality')
    |     -> <function Entry.accept at 0x7fea67f467b8>
    -> <Entry(title=Lost S01 720p hdtv h264,state=accepted)>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/entry.py", line 177, in accept
    self.run_hooks('accept', reason=reason, **kwargs)
    |    |                          |         -> {}
    |    |                          -> 'target quality'
    |    -> <function Entry.run_hooks at 0x7fea67f46488>
    -> <Entry(title=Lost S01 720p hdtv h264,state=accepted)>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/entry.py", line 118, in run_hooks
    func(self, **kwargs)
    |    |       -> {'reason': 'target quality'}
    |    -> <Entry(title=Lost S01 720p hdtv h264,state=accepted)>
    -> <unprintable partial object>

TypeError: _exclude_season_on_accept() got multiple values for argument 'series_entity'
2021-02-11 22:54:48 CRITICAL manager       xTV_Test_season_pack_2 An unexpected crash has occurred. Writing crash report to /home/downloader/.config/flexget/crash_report.2021.02.11.225448995537.log. Please verify you are running the latest version of flexget by using "flexget -V" from CLI or by using version_checker plugin at http://flexget.com/wiki/Plugins/version_checker. You are currently using version 3.1.89
2021-02-11 22:54:48 DEBUG    manager       xTV_Test_season_pack_2 Traceback:
Traceback (most recent call last):

  File "/usr/lib/python3.7/threading.py", line 885, in _bootstrap
    self._bootstrap_inner()
    |    -> <function Thread._bootstrap_inner at 0x7fea69b0ce18>
    -> <Thread(task_queue, started daemon 140644677314304)>
  File "/usr/lib/python3.7/threading.py", line 917, in _bootstrap_inner
    self.run()
    |    -> <function Thread.run at 0x7fea69b0cbf8>
    -> <Thread(task_queue, started daemon 140644677314304)>
  File "/usr/lib/python3.7/threading.py", line 865, in run
    self._target(*self._args, **self._kwargs)
    |    |        |    |        |    -> {}
    |    |        |    |        -> <Thread(task_queue, started daemon 140644677314304)>
    |    |        |    -> ()
    |    |        -> <Thread(task_queue, started daemon 140644677314304)>
    |    -> <bound method TaskQueue.run of <flexget.task_queue.TaskQueue object at 0x7fea64965080>>
    -> <Thread(task_queue, started daemon 140644677314304)>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/task_queue.py", line 46, in run
    self.current_task.execute()
    |    |            -> <function Task.execute at 0x7fea67d610d0>
    |    -> <flexget.task.Task object at 0x7fea64795a20>
    -> <flexget.task_queue.TaskQueue object at 0x7fea64965080>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/task.py", line 87, in wrapper
    return func(self, *args, **kw)
           |    |      |       -> {}
           |    |      -> ()
           |    -> <flexget.task.Task object at 0x7fea64795a20>
           -> <function Task.execute at 0x7fea67d61048>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/task.py", line 722, in execute
    self._execute()
    |    -> <function Task._execute at 0x7fea67d60f28>
    -> <flexget.task.Task object at 0x7fea64795a20>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/task.py", line 688, in _execute
    self.__run_task_phase(phase)
    |                     -> 'filter'
    -> <flexget.task.Task object at 0x7fea64795a20>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/task.py", line 514, in __run_task_phase
    response = self.__run_plugin(plugin, phase, args)
               |                 |       |      -> (<flexget.task.Task object at 0x7fea64795a20>, [{'Lost (2004)': {'identified_by': 'ep', 'quality': '720p|1080p webrip+', 'tim...
               |                 |       -> 'filter'
               |                 -> <PluginInfo(name=series)>
               -> <flexget.task.Task object at 0x7fea64795a20>
> File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/task.py", line 547, in __run_plugin
    result = method(*args, **kwargs)
             |       |       -> {}
             |       -> (<flexget.task.Task object at 0x7fea64795a20>, [{'Lost (2004)': {'identified_by': 'ep', 'quality': '720p|1080p webrip+', 'tim...
             -> <Event(name=plugin.series.filter,func=on_task_filter,priority=128)>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/event.py", line 20, in __call__
    return self.func(*args, **kwargs)
           |    |     |       -> {}
           |    |     -> (<flexget.task.Task object at 0x7fea64795a20>, [{'Lost (2004)': {'identified_by': 'ep', 'quality': '720p|1080p webrip+', 'tim...
           |    -> <bound method FilterSeries.on_task_filter of <flexget.components.series.series.FilterSeries object at 0x7fea64a541d0>>
           -> <Event(name=plugin.series.filter,func=on_task_filter,priority=128)>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/components/series/series.py", line 562, in on_task_filter
    self.process_series(task, series_entries, series_config)
    |    |              |     |               -> {'identified_by': 'ep', 'quality': '720p|1080p webrip+', 'timeframe': '4 hours', 'target': '720p webrip+ h264+', 'season_pack...
    |    |              |     -> <unprintable dict object>
    |    |              -> <flexget.task.Task object at 0x7fea64795a20>
    |    -> <function FilterSeries.process_series at 0x7fea64cb8a60>
    -> <flexget.components.series.series.FilterSeries object at 0x7fea64a541d0>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/components/series/series.py", line 782, in process_series
    if self.process_timeframe_target(config, entries, downloaded):
       |    |                        |       |        -> []
       |    |                        |       -> [<Entry(title=Lost S01 720p hdtv h264,state=accepted)>]
       |    |                        -> {'identified_by': 'ep', 'quality': '720p|1080p webrip+', 'timeframe': '4 hours', 'target': '720p webrip+ h264+', 'season_pack...
       |    -> <function FilterSeries.process_timeframe_target at 0x7fea64cb8b70>
       -> <flexget.components.series.series.FilterSeries object at 0x7fea64a541d0>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/components/series/series.py", line 876, in process_timeframe_target
    entry.accept('target quality')
    |     -> <function Entry.accept at 0x7fea67f467b8>
    -> <Entry(title=Lost S01 720p hdtv h264,state=accepted)>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/entry.py", line 177, in accept
    self.run_hooks('accept', reason=reason, **kwargs)
    |    |                          |         -> {}
    |    |                          -> 'target quality'
    |    -> <function Entry.run_hooks at 0x7fea67f46488>
    -> <Entry(title=Lost S01 720p hdtv h264,state=accepted)>
  File "/home/downloader/flexget/lib/python3.7/site-packages/flexget/entry.py", line 118, in run_hooks
    func(self, **kwargs)
    |    |       -> {'reason': 'target quality'}
    |    -> <Entry(title=Lost S01 720p hdtv h264,state=accepted)>
    -> <unprintable partial object>

TypeError: _exclude_season_on_accept() got multiple values for argument 'series_entity'

@liiight
Copy link
Copy Markdown
Member Author

liiight commented Feb 15, 2021

@soloam try now
@gazpachoking the original failure originated due the way hooks are invoked:

    def run_hooks(self, action: str, **kwargs) -> None:
        """
        Run hooks that have been registered for given ``action``.

        :param action: Name of action to run hooks for
        :param kwargs: Keyword arguments that should be passed to the registered functions
        """
        for func in self._hooks[action]:
            func(self, **kwargs)

This means that each added hook needs to accept self and any additional kwargs, regardless of its functionality or the signature won't match. Not a big deal though, but kinda unexcpected.

@soloam
Copy link
Copy Markdown
Contributor

soloam commented Feb 15, 2021

I tested the new commits and seems to be working OK :)

Nice job, thank you! 👍

@gazpachoking
Copy link
Copy Markdown
Member

Not 100% sure what the exact issue is. Can we add a test into the suite that shows the problem?

@soloam
Copy link
Copy Markdown
Contributor

soloam commented Feb 15, 2021

Not 100% sure what the exact issue is. Can we add a test into the suite that shows the problem?

Please look at the error report in #2849

I have test scenario... The problem seams solved by this PR

Thank You

@liiight
Copy link
Copy Markdown
Member Author

liiight commented Feb 16, 2021

Not 100% sure what the exact issue is. Can we add a test into the suite that shows the problem?

Yeah sure, I was trying to get away with being lazy 😋

@gazpachoking
Copy link
Copy Markdown
Member

I'm a bit concerned about using an entry hook here. We are hooking the acceptance done by the same plugin, right? It also feels weird that we now might be recording the downloads in two different places/phases depending on options. Would be really nice if we could find a nicer way. (Though I could be convinced if there isn't one, as long as we add a test.) If we get a test in there I might have a play myself to see what other methods might be possible.

@liiight
Copy link
Copy Markdown
Member Author

liiight commented Feb 16, 2021

First off, sure, I'll definitely add a test, I didnt have the time to do this yet.

However, I think that due to the way that series processing is implemented, an entry hook is an elegant solution here.

While iterating over series entities/entries, some logic in the loop rejects and continue but other functions (like proper and timeframes handling) accept then continue. That's what caused the bug, I initially added the accepted season to the exclusion list at the end of the loop, assuming that only and all non rejected entries get to that point, which isn't correct.

So a hook fits perfectly here IMO as now the exclusion list gets populated regardless of the phase or code point in which the series entry is accepted.

@gazpachoking
Copy link
Copy Markdown
Member

You could be totally right on that. I know the interplay between all those options is already complicated, just hoping we aren't making it more complicated by adding extra paths.

@soloam
Copy link
Copy Markdown
Contributor

soloam commented Feb 16, 2021

Thank you all for the time taken with this! Season Pack is a great addition, but with this bug makes everything more complex, because I can't rely on the season pack to reject entries in the same task, I have to split all the tasks in 2, 1 for the episode, and other for the season pack, with in a config so big like mine (a lot of trackers) makes everything a lot more complex. Since I added the code from @liiight to my flexget, all seams ok, the seasons and episodes seem to be managed correctly.

@liiight
Copy link
Copy Markdown
Member Author

liiight commented Feb 22, 2021

just hoping we aren't making it more complicated by adding extra paths

I think we're keeping the level of complexity as high as it was 😝

@soloam
Copy link
Copy Markdown
Contributor

soloam commented Feb 26, 2021

I have reasons to believe that might still exit a problem in this! I have your code working on my main flexget and I just had a Pack being downloaded after the episodes! But this time in 2 different tasks! From what I can tell from the Logs:

  • System found the pack in one task but was 1080p so the timeframe made a hold on the download
  • In the next task e found the episodes in 720p, so he made the download
  • 4 hours passed (my timeframe) and the system downloaded the first pack

I will check my config and try to replicate this, and see if the problem is related to my config or some bug!

I'll report back again

@soloam
Copy link
Copy Markdown
Contributor

soloam commented Feb 26, 2021

Consider my previews post without effect, this was configuration error in my yaml! I was allowing to download packs, even with episodes already downloaded! Your code is working ok!

@liiight liiight changed the title Added hook to add season to accepted season on any entry acceptance [fix] series - Added hook to add season to accepted season on any entry acceptance Feb 27, 2021
@liiight liiight merged commit 7c334a8 into develop Feb 27, 2021
@liiight liiight deleted the fix_accepted_season_pack branch February 27, 2021 14:27
@liiight
Copy link
Copy Markdown
Member Author

liiight commented Feb 27, 2021

@gazpachoking i merged this to avoid it getting stale

@soloam
Copy link
Copy Markdown
Contributor

soloam commented Feb 27, 2021

Thank you for the time spend on this!

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 this pull request may close these issues.

season_packs doesn't reject episodes if in the same feed and with target and timeframe

3 participants