MycroftAI / mycroft-core

Mycroft Core, the Mycroft Artificial Intelligence platform.
https://mycroft.ai
Apache License 2.0
6.51k stars 1.27k forks source link

Padatious doesn't need to run 3 times #3003

Closed Joanguitar closed 2 years ago

Joanguitar commented 2 years ago

Description

New class PadatiousMatcher created to avoid running Padatious intent service 3 times when a low priority callback happens. Instead, the new class stores the result from the first call and uses it on subsequent calls with different match priority.

How to test

There's no new feature, the way to see its impact is by getting faster low priority fallbacks. So just say something to Mycroft that it shouldn't understand and see that it takes shorter for it to reply.

forslund commented 2 years ago

Nice! I think the lru_cache decorator here can be removed with this since that was intended to handle the caching of the result to avoid the re-runs. Or are both needed?

codecov-commenter commented 2 years ago

Codecov Report

Merging #3003 (9a87f1d) into dev (3c76177) will increase coverage by 0.01%. The diff coverage is 80.00%.

:exclamation: Current head 9a87f1d differs from pull request most recent head 0f98c56. Consider uploading reports for the commit 0f98c56 to get more accurate results Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #3003      +/-   ##
==========================================
+ Coverage   52.80%   52.82%   +0.01%     
==========================================
  Files         123      123              
  Lines       11106    11117      +11     
==========================================
+ Hits         5865     5873       +8     
- Misses       5241     5244       +3     
Impacted Files Coverage Δ
mycroft/skills/intent_service.py 67.56% <50.00%> (-0.16%) :arrow_down:
...ycroft/skills/intent_services/padatious_service.py 55.00% <81.25%> (+1.84%) :arrow_up:
mycroft/skills/intent_services/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3c76177...0f98c56. Read the comment docs.

Joanguitar commented 2 years ago

Nice! I think the lru_cache decorator here can be removed with this since that was intended to handle the caching of the result to avoid the re-runs. Or are both needed?

True, I just removed it.

devops-mycroft commented 2 years ago

Voight Kampff Integration Test Succeeded (Results)