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

Fixed failing HTTP request when synthesizing TextToSpeech #194

Merged

Conversation

3 participants
@teddynewell
Copy link
Contributor

commented Mar 10, 2016

What's this PR do?

Fixes TextToSpeech synthesis so the QuickStart example code works.

  • Fixes the failing HTTP request in TextToSpeech().synthesize(_:voice:completionHandler) by removing the unneeded JSON content-type header. Since no HTTP message is included in the request body, the content-type is superfluous.
    • Note: An extraneous content-type probably shouldn't cause a 400 response when the request body is empty for a GET request.

- Updates AlamofireObjectMapper version for Carthage compatibility and to fix Carthage build error due to deployment targets without introducing any API changes.
- Uses ~> for maximum compatibility in the dependency graph

Relevant Issues

Resolves #193

Fixed failing HTTP request when synthesizing TextToSpeech by removing…
… unneeded content-type header

- Fixed the failing HTTP request in `TextToSpeech().synthesize(_:voice:completionHandler)` by removing the unneeded JSON content-type header. Since no HTTP message is included in the request body, the content-type is superfluous.
  - Note that an extraneous content-type probably shouldn't cause a 400 response when the request body is empty for a GET request.

- Updated AlamofireObjectMapper version for Carthage compatibility and to fix [Carthage build error due to deployment targets](tristanhimmelman/AlamofireObjectMapper#63) without introducing any API changes.
  - Uses `~>` for maximum compatibility in the dependency graph

@teddynewell teddynewell changed the title Fixed failing HTTP request when synthesizing TextToSpeech by removing… Fixed failing HTTP request when synthesizing TextToSpeech Mar 10, 2016

@teddynewell

This comment has been minimized.

Copy link
Contributor Author

commented Mar 10, 2016

I made this small PR against master so it can be deployed as a hotfix @glennrfisher. I see there may be legal issues about updating dependency releases, so let me know if I should drop the Cartfile changes.

@kasunk

This comment has been minimized.

Copy link

commented Mar 10, 2016

Hi @teddynewell,

I had the same issue you had, Even thought I added your fixes and rebuild again it doesn't work for me.

@glennrfisher

This comment has been minimized.

Copy link
Contributor

commented Mar 10, 2016

You're very right! The problem is with the contentType: .JSON line. I believe the Text to Speech service was updated to be stricter about requests.

I did commit a fix to the develop branch, but unfortunately we have code in develop that is not yet ready for master. So my commit is just waiting there, doing nobody any good.

The legal questions make me hesitant to merge your Cartfile updates until we learn more--even though they are much needed and long overdue. Can you remove the changes to Cartfile and Cartfile.resolved so that the only changes are to TextToSpeech.swift?

Thanks @teddynewell! Really appreciate your work. :)

Reverses changes to AlamofireObjectMapper version
Reverses changes to AlamofireObjectMapper version in `Cartfile` and
`Cartfile.resolved` to keep this PR to-the-point.
@teddynewell

This comment has been minimized.

Copy link
Contributor Author

commented Mar 10, 2016

@glennrfisher reversed the changes to Cartfile and Cartfile.resolved so this PR only changes TextToSpeech.swift.

@glennrfisher

This comment has been minimized.

Copy link
Contributor

commented Mar 10, 2016

Thanks, @teddynewell! Much appreciated.

glennrfisher added a commit that referenced this pull request Mar 10, 2016

Merge pull request #194 from teddynewell/textToSpeechFix
Fixed failing HTTP request when synthesizing TextToSpeech

@glennrfisher glennrfisher merged commit ee74c3f into watson-developer-cloud:master Mar 10, 2016

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@glennrfisher

This comment has been minimized.

Copy link
Contributor

commented Mar 11, 2016

Forgot to push a new release yesterday. I tagged your fix and labeled it v0.1.2. It should now be accessible via Carthage.

@teddynewell

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2016

Thank you @glennrfisher ! And good to hear about upcoming SpeechToText improvements – we are evaluating that service as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.