[change] estimate_release - make configurable #3650
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 __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. 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. Interesting, this is similar to how the There was a problem hiding this comment. 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. 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. 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.
Yeah, I love this idea. There was a problem hiding this comment. 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. | ||
| """ | ||
| 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(f"estimating release date for {entry['title']}") | ||
| for estimator in self.get_estimators(): | ||
| estimate = estimator(entry) | ||
| # 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) | ||
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.