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

Amazon Polly as a provider for the text-to-speech feature. #734

Merged
merged 14 commits into from
Apr 1, 2024

Conversation

iamdharmesh
Copy link
Member

@iamdharmesh iamdharmesh commented Mar 4, 2024

Description of the Change

PR Adds Amazon Polly as a provider for the Text-to-speech feature.

Closes #728

@jeffpaul @dkotter Amazon Polly provides additional options such as Newscaster Speaking Style and SSML, which offers features like including breathing sounds and emphasizing specific words or phrases. I haven't implemented it in this PR, but we can consider integrating it in the future if there are specific client requirements around these features.

@dkotter, I haven't added E2E tests for this because I haven't figured out yet how to mock the API, given that we are using the AWS PHP SDK here. Please let me know if you have any ideas on this.

How to test the Change

  1. Go to Tools > ClassifAI > Language Processing > Text to Speech.
  2. Select the "Amazon Polly" option as the provider.
  3. Add AWS credentials and save settings.
  4. Create/Edit a post and ensure that the Text-to-Speech feature is working as expected.

Changelog Entry

Added - Amazon Polly as a provider for the text-to-speech feature.

Credits

Props @jeffpaul @iamdharmesh

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@iamdharmesh iamdharmesh self-assigned this Mar 4, 2024
@github-actions github-actions bot added this to the 3.1.0 milestone Mar 4, 2024
@iamdharmesh iamdharmesh changed the title [WIP] Add Amazon Polly provider for Text to speech feature Amazon Polly as a provider for the text-to-speech feature. Mar 8, 2024
@iamdharmesh iamdharmesh marked this pull request as ready for review March 8, 2024 12:13
@iamdharmesh iamdharmesh requested review from dkotter, jeffpaul and a team as code owners March 8, 2024 12:13
@github-actions github-actions bot added the needs:code-review This requires code review. label Mar 8, 2024
@dkotter
Copy link
Collaborator

dkotter commented Mar 14, 2024

@dkotter, I haven't added E2E tests for this because I haven't figured out yet how to mock the API, given that we are using the AWS PHP SDK here. Please let me know if you have any ideas on this.

I guess my first question would be do we need to use the SDK here? I know that can help simplify things but we haven't used any SDKs for the other Providers up to this point. I'm not opposed to it, just wondering if there was a specific reason.

But I can think of two approaches we can take to mock the requests:

  1. Add a short-circuit filter right before we make the request to AWS, allowing us to return our own results. This is basically what WordPress does, my only concern is we'd basically be adding a filter for testing purposes only which I don't love
  2. Because the main request goes through a custom REST endpoint, there is a filter there that fires before any callbacks are called: rest_pre_dispatch. We could use this and return a hardcoded result, similar to how we're currently using the pre_http_request filter. This wouldn't work for all scenarios (like triggering Text to Speech from the inline row action) but should work to test the main use case of publishing content

@iamdharmesh
Copy link
Member Author

I guess my first question would be do we need to use the SDK here? I know that can help simplify things but we haven't used any SDKs for the other Providers up to this point. I'm not opposed to it, just wondering if there was a specific reason.

The main reason for using the SDK was to keep things simple, especially concerning signing and authenticating REST requests. I believe we don't have this complex authentication process with existing providers. I'm open to getting rid of the SDK here and writing a custom class for handling authentication and REST operations (similar to what we did for the OpenAPI). Please let me know if you think we should remove the SDK here.

I can think of two approaches we can take to mock the requests:

Approach #1 seems like the better choice to me as it allows us to cover all scenarios.

Thanks

@dkotter
Copy link
Collaborator

dkotter commented Mar 19, 2024

The main reason for using the SDK was to keep things simple, especially concerning signing and authenticating REST requests. I believe we don't have this complex authentication process with existing providers. I'm open to getting rid of the SDK here and writing a custom class for handling authentication and REST operations (similar to what we did for the OpenAPI). Please let me know if you think we should remove the SDK here.

I think we're fine to proceed with keeping the SDK here. It does increase the size of the final release zip due to all the code the SDK brings but that should be fine.

Approach #1 seems like the better choice to me as it allows us to cover all scenarios.

That works for me

dkotter
dkotter previously approved these changes Mar 29, 2024
includes/Classifai/Providers/AWS/AmazonPolly.php Outdated Show resolved Hide resolved
includes/Classifai/Providers/AWS/AmazonPolly.php Outdated Show resolved Hide resolved
@dkotter dkotter merged commit fe28ee7 into develop Apr 1, 2024
13 checks passed
@dkotter dkotter deleted the enhancement/728 branch April 1, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs:code-review This requires code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Amazon Polly as a Provider for Text to speech feature.
2 participants