Skip to content
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

Consolidate TTS cache logic #2853

Merged
merged 9 commits into from Mar 11, 2021
Merged

Conversation

chrisveilleux
Copy link
Member

@chrisveilleux chrisveilleux commented Mar 10, 2021

Description

The impetus for this change is an attempt to fix an issue with pre-loaded cache files on devices that use Mimic2.

  • Consolidate all the logic dealing with TTS caching into a single module.
  • Replace logic that copies persistent cache to temporary cache with logic that checks both places

How to test

Check that persistent cache files are not in /tmp and still get used when requested.

Contributor license agreement signed?

CLA [Y]

@pep8speaks
Copy link

pep8speaks commented Mar 10, 2021

Hello @chrisveilleux! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-03-11 03:16:44 UTC

@devops-mycroft devops-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Mar 10, 2021
@chrisveilleux chrisveilleux changed the title Feature/consolidate tts cache Consolidate TTS cache logic Mar 10, 2021
mycroft/tts/cache.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@forslund forslund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good. Great to get rid of the copy from persistent cache to the tmp one.

I noted two things (one that is an iseue, and one that may just be me not quite grasping the idea)

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

2 similar comments
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

1 similar comment
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@chrisveilleux chrisveilleux merged commit bf2670c into dev Mar 11, 2021
@chrisveilleux chrisveilleux deleted the feature/consolidate_tts_cache branch March 11, 2021 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants