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

Update speech options to use credentials #2759

Merged
merged 5 commits into from
Jan 15, 2020
Merged

Conversation

compulim
Copy link
Contributor

@compulim compulim commented Dec 18, 2019

Fixes #2459.

Changelog Entry

Fixed

  • Fixes #2459. Updated Cognitive Services Speech Services to use latest fetch credentials signature, by @compulim in PR #2740

Description

Updated to use credentials from web-speech-cognitive-services. Deprecating authorizationToken, region, and subscriptionKey.

Specific Changes

  • Update createCognitiveServicesSpeechServicesPonyfillFactory to use credentials in favor of authorizationToken, region, and subscriptionKey
  • Add deprecation notes to outdated options
  • Update all samples to use credentials

  • Testing Added
    • We currently do not have test for the ponyfill factory. We should port the test harness from DLSpeech SDK.

@compulim compulim changed the title Update speech options to use credentials [DRAFT] Update speech options to use credentials Dec 18, 2019
@compulim compulim changed the title [DRAFT] Update speech options to use credentials Update speech options to use credentials Dec 18, 2019
@compulim compulim marked this pull request as ready for review December 18, 2019 20:39
@compulim
Copy link
Contributor Author

@tonyanziano this is the PR for having both authorizationToken and region in a single async function.

@coveralls
Copy link

coveralls commented Dec 18, 2019

Coverage Status

Coverage decreased (-0.2%) to 65.684% when pulling ce8e63e on compulim:fix-2459 into 6addf73 on microsoft:master.

@corinagum
Copy link
Contributor

When I try to run the samples locally, I get the following error:
image

getting started full bundle sample does not have the same error

Copy link
Contributor

@corinagum corinagum left a comment

Choose a reason for hiding this comment

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

Please fix samples.

(can you please answer my dumb question? haha)

@compulim
Copy link
Contributor Author

compulim commented Jan 14, 2020

@corinagum I tried both 01/a.full-bundle and 03.speech/b.cognitive-speech-services-js loading locally-built webchat.js and it looks good to me on Chromium Edge. Probably fixed.

image

@corinagum corinagum merged commit b9612d5 into microsoft:master Jan 15, 2020
@rvallireply
Copy link

Hello, since a few hours we are getting an error using webchat with cognitive speech.
Maybe related to this commit?
See #2802 (comment)

@compulim
Copy link
Contributor Author

@rvallireply for #2802, it is unrelated to this commit. It is a service-side issue.

For this PR, we do found a bug #2822 and is going to fix it in #2824.

If you see further issues, could you open a new issue? We might miss your comment in this PR because it is already merged and closed.

@compulim compulim mentioned this pull request Mar 5, 2020
40 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Tracking] Update webSpeechPonyFillFactory signature to accept a promise for region
4 participants