Skip to content

Update speech options to use credentials#2759

Merged
corinagum merged 5 commits intomicrosoft:masterfrom
compulim:fix-2459
Jan 15, 2020
Merged

Update speech options to use credentials#2759
corinagum merged 5 commits intomicrosoft:masterfrom
compulim:fix-2459

Conversation

@compulim
Copy link
Copy Markdown
Contributor

@compulim compulim commented Dec 18, 2019 •
edited
Loading

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
Copy Markdown
Contributor Author

compulim commented Dec 18, 2019

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

@coveralls
Copy link
Copy Markdown

coveralls commented Dec 18, 2019 •
edited
Loading

Coverage Status

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

corinagum
corinagum reviewed Jan 10, 2020
View reviewed changes
Comment thread packages/bundle/src/createCognitiveServicesSpeechServicesPonyfillFactory.js Outdated
@corinagum
Copy link
Copy Markdown
Contributor

corinagum commented Jan 10, 2020

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

corinagum
corinagum reviewed Jan 10, 2020
View reviewed changes
Comment thread packages/bundle/src/createCognitiveServicesSpeechServicesPonyfillFactory.js
corinagum
corinagum suggested changes Jan 10, 2020
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

compulim commented Jan 14, 2020 •
edited
Loading

@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
Copy Markdown

rvallireply commented Jan 15, 2020

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

This was referenced Jan 16, 2020
Closed
Merged
@compulim compulim deleted the fix-2459 branch January 16, 2020 20:26
@compulim
Copy link
Copy Markdown
Contributor Author

compulim commented Jan 16, 2020

@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
Closed
40 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@a-b-r-o-w-n a-b-r-o-w-n Awaiting requested review from a-b-r-o-w-n a-b-r-o-w-n is a code owner

@cwhitten cwhitten Awaiting requested review from cwhitten cwhitten is a code owner

@srinaath srinaath Awaiting requested review from srinaath srinaath is a code owner

@tdurnford tdurnford Awaiting requested review from tdurnford tdurnford is a code owner

@tonyanziano tonyanziano Awaiting requested review from tonyanziano

1 more reviewer

@corinagum corinagum corinagum approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

4 participants

@compulim @coveralls @corinagum @rvallireply