Skip to content
This repository has been archived by the owner on Sep 8, 2024. It is now read-only.

Add configurable API url to Watson TTS #2514

Merged
merged 3 commits into from
Mar 26, 2020
Merged

Add configurable API url to Watson TTS #2514

merged 3 commits into from
Mar 26, 2020

Conversation

krisgesling
Copy link
Contributor

Description

The new IBM Watson TTS service now provides a unique url for your TTS
API resource. This adds a configuration option to override the default
url. As the url provided by IBM contains the api_path, we check for it
and if found remove it, as the RemoteTTS.__request method appends it
when making the call.

How to test

Create an IBM cloud account and add Watson TTS as described in the docs.
Add a url attribute to your config using the url provided with your API key. This can be added with or without the API path "/v1/synthesize", and with or without trailing slashes.

Contributor license agreement signed?

The new IBM Watson TTS service now provides a unique url for your TTS
API resource. This adds a configuration option to override the default
url. As the url provided by IBM contains the api_path, we check for it 
and if found remove it, as the RemoteTTS.__request method appends it 
when making the call.
@devs-mycroft devs-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Mar 26, 2020
@pep8speaks
Copy link

pep8speaks commented Mar 26, 2020

Hello @krisgesling! 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 2020-03-26 06:13:04 UTC

@forslund
Copy link
Collaborator

The remove_trailing_slash is suggested for removal in #2496, so maybe replace that with a simpler `.rstrip('/')

I also wonder if we want to do some generalization, allowing all remoteTTS to specify a custom url using the same config parameter, by changing the self.url = url line in remote_tts.py to self.url = config.get("url", self.url).

I'm also wondering if the remoteTTS could use some extra customizability... but I'll add a separate issue for that.

@krisgesling
Copy link
Contributor Author

Yeah good suggestion re generalization. This seems like a good candidate.

Wondering if it's worth checking for the url.endswith(api_path) in remote_tts.py too?
The only case it would break for is if an API was in the form api.com/endpoint/endpoint
hence the url == 'api.com/endpoint',
and api_path == '/endpoint'
and the url would then get stripped to become just api.com

This seems like a very remote case.

@krisgesling
Copy link
Contributor Author

Maybe I'll leave it where it is for now, and as you said we can consider further changes to remote_tts in a later PR.

@forslund
Copy link
Collaborator

Looks great!

@forslund forslund merged commit 13defa1 into dev Mar 26, 2020
@forslund forslund deleted the bugfix/watson-tts branch March 26, 2020 17:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.

4 participants