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
[change] estimate_release - make configurable #3650
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,31 @@ | ||
from datetime import datetime | ||
from typing import Any, Dict, List, Optional, Union | ||
|
||
from loguru import logger | ||
|
||
from flexget import plugin | ||
from flexget.event import event | ||
from flexget.plugin import PluginInfo | ||
from flexget.task import Task | ||
from flexget.utils.tools import parse_timedelta | ||
|
||
logger = logger.bind(name='est_released') | ||
ESTIMATOR_INTERFACE = "estimate_release" | ||
|
||
# Mapping of available estimate providers to plugin instance | ||
estimators = {} | ||
# Task specific estimate configuration | ||
task_estimate_config: Dict[str, Any] = {} | ||
|
||
# We need to wait until manager startup to access other plugin instances, to make sure they have all been loaded | ||
@event('manager.startup') | ||
def init_estimators(manager) -> None: | ||
"""Prepare the list of available estimator plugins.""" | ||
|
||
for provider in plugin.get_plugins(interface=ESTIMATOR_INTERFACE): | ||
estimators[provider.name.replace('est_', '')] = provider | ||
|
||
logger.debug('setting default estimators to {}', list(estimators.keys())) | ||
|
||
|
||
class EstimateRelease: | ||
|
@@ -12,32 +34,105 @@ class EstimateRelease: | |
for various things (series, movies). | ||
""" | ||
|
||
def estimate(self, entry): | ||
def __init__(self): | ||
self.task_estimate_config = {} | ||
|
||
@property | ||
def schema(self) -> Dict[str, Any]: | ||
"""Create schema that allows configuring estimator providers and | ||
related settings. | ||
""" | ||
|
||
schema = { | ||
'type': 'object', | ||
'properties': { | ||
'providers': { | ||
'type': 'array', | ||
'items': { | ||
'type': 'string', | ||
'enum': [ | ||
p.name.replace('est_', '') | ||
for p in plugin.get_plugins(interface=ESTIMATOR_INTERFACE) | ||
], | ||
}, | ||
}, | ||
'offset': {'type': 'string', 'format': 'interval', 'default': '0 days'}, | ||
}, | ||
'additionalProperties': False, | ||
} | ||
|
||
return schema | ||
|
||
def on_task_start(self, task: Task, config) -> None: | ||
# Load task specific estimator configuration | ||
if config: | ||
self.task_estimate_config = config | ||
|
||
def on_task_exit(self, task: Task, config) -> None: | ||
# Restore default estimator configuration for next task run | ||
self.task_estimate_config = {} | ||
Comment on lines
+66
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This modifying configuration and restoring later is not a good idea. Generally speaking plugins should be stateless and not rely on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, this is similar to how the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, apologies. I guess there's no better way for it in these kinds of situations then. There is still issue with having There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, we have to do some hacky stuff for bits that don't directly get a task parameter, there isn't a better way at the moment. I was thinking of making a plugin method that worked like a context manager, to make plugins that do setup and teardown around tasks a bit easier. Something like: def on_task_context(self, task, config):
if config:
self.task_estimate_config = config
try:
yield
finally:
self.task_estimate_config = {} That way you don't have to hook task_start, task_exit, and task_abort trying to manage state. @paranoidi Thoughts? I also had this crazier idea based on that, but my gut tells me it's probably bad. It would allow a plugin to retain state during a task without storing things to self. def on_task_phases(self, task, config):
yield 'start'
setupstuff = somefunc()
yield 'filter'
for entry in task.all_entries:
do_something(entry, setupstuff)
yield 'exit'
teardown(setupstuff) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Heh, sure. I have some thoughts. Not overly sold on the idea. I've been thinking years that we should have a baseclass for plugins and in there have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah, I love this idea. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe the baseclass could introduce setup/teardown methods (although I kinda dislike those names as they are heavily unit test related) that are guaranteed to run as expected. It would be much more explicit than relying on on_task_start etc. |
||
|
||
on_task_abort = on_task_exit | ||
|
||
def get_estimators(self) -> List[PluginInfo]: | ||
""" | ||
Returns the list of configured estimator providers for the task. If no | ||
providers are configured, all available providers are returned. | ||
|
||
Providers are sorted by the plugin priority. | ||
|
||
:return: Sorted list of estimator plugin instances. | ||
""" | ||
if "providers" in self.task_estimate_config: | ||
# Map task configured providers to plugin instance map | ||
try: | ||
task_estimators = [ | ||
estimators[p].instance.estimate for p in self.task_estimate_config['providers'] | ||
] | ||
except KeyError as error: | ||
logger.error(f"invalid provider plugin given: {error}") | ||
raise | ||
else: | ||
# Use all loaded estimator plugins | ||
task_estimators = [e.instance.estimate for e in estimators.values()] | ||
|
||
return sorted( | ||
task_estimators, | ||
key=lambda e: getattr(e, 'priority', plugin.PRIORITY_DEFAULT), | ||
reverse=True, | ||
) | ||
|
||
@property | ||
def offset(self) -> str: | ||
""" | ||
Return the configured offset for the task. | ||
""" | ||
Estimate release schedule for Entry | ||
return self.task_estimate_config.get('offset', '0 days') | ||
|
||
def estimate(self, entry) -> Dict[str, Union[bool, Optional[datetime]]]: | ||
""" | ||
Estimate release schedule for entry | ||
|
||
:param entry: | ||
:return: estimated date of released for the entry, None if it can't figure it out | ||
""" | ||
|
||
logger.debug(entry['title']) | ||
estimators = [ | ||
e.instance.estimate for e in plugin.get_plugins(interface='estimate_release') | ||
] | ||
for estimator in sorted( | ||
estimators, key=lambda e: getattr(e, 'priority', plugin.PRIORITY_DEFAULT), reverse=True | ||
): | ||
logger.debug(f"estimating release date for {entry['title']}") | ||
for estimator in self.get_estimators(): | ||
estimate = estimator(entry) | ||
# return first successful estimation | ||
# Return first successful estimation | ||
if estimate is not None: | ||
estimation = estimate | ||
break | ||
else: | ||
estimation = {'data_exists': False, 'entity_date': None} | ||
|
||
if estimation['entity_date']: | ||
estimation['entity_date'] = estimation['entity_date'] + parse_timedelta(self.offset) | ||
|
||
return estimation | ||
|
||
|
||
@event('plugin.register') | ||
def register_plugin(): | ||
plugin.register(EstimateRelease, 'estimate_release', api_ver=2, interfaces=[]) | ||
plugin.register(EstimateRelease, 'estimate_release', api_ver=2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the way to handle sub plugins & interfaces, please look at how discover plugin handles search interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I'll take a look.