fallback order override #987
Conversation
|
@JarbasAI thank you for the contribution, travis is getting those pep8 erros |
penrods
added
the
CLA: Yes
mycroft/skills/core.py
Outdated
| @@ -401,6 +401,9 @@ def shutdown(self): | |||
|
|
|||
| class FallbackSkill(MycroftSkill): | |||
| fallback_handlers = {} | |||
| folders = {} | |||
| override = skills_config.get("fallback_override", False) | |||
| order = skills_config.get("fallback_priority", []) | |||
There was a problem hiding this comment.
is the override config parameter necessary. The check could be if len(order) > 0
There was a problem hiding this comment.
i like the override parameter, i may want to have the override list there but only toggle it sometimes, with the PR for changing config at runtime from skills this can even be changed vocally
mycroft/skills/core.py
Outdated
| @@ -413,20 +416,58 @@ def make_intent_failure_handler(cls, ws): | |||
| """Goes through all fallback handlers until one returns true""" | |||
|
|
|||
| def handler(message): | |||
| for _, handler in sorted(cls.fallback_handlers.items(), | |||
There was a problem hiding this comment.
split this into ordered_handler using the handler order from config and priority_handler for the default and do the check at L467 (returning either ordered_handler or priority_handler)
mycroft/skills/core.py
Outdated
| flag2 = True | ||
|
|
||
| if not flag1: | ||
| logger.warn('Could not remove fallback!') |
There was a problem hiding this comment.
maybe call the flag success and move the waring text to just after the for loop where it's used (L517)
|
changes made |
|
Hello @JarbasAI! Thanks for updating the PR.
|
sometimes we may not agree with dev choices on fallback priority, an option to override the fallback order in config would be good, so we can get updates without git pull complaining of stashing changes, also good for dev purposes
This pr is more for discussion than a serious proposal