[fix] series - Added hook to add season to accepted season on any entry acceptance#2851
[fix] series - Added hook to add season to accepted season on any entry acceptance#2851
Conversation
|
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 |
|
@soloam try now 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 |
|
I tested the new commits and seems to be working OK :) Nice job, thank you! 👍 |
|
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 |
Yeah sure, I was trying to get away with being lazy 😋 |
|
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. |
|
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. |
|
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. |
|
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. |
I think we're keeping the level of complexity as high as it was 😝 |
|
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:
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 |
|
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! |
|
@gazpachoking i merged this to avoid it getting stale |
|
Thank you for the time spend on this! |
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:
Addressed issues: