-
Notifications
You must be signed in to change notification settings - Fork 925
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
feat(azure): Realtime API support #1287
Conversation
a28dbcc
to
e90657b
Compare
d1d50c8
to
fb61fc2
Compare
e90657b
to
f2e649f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
@@ -9,7 +9,7 @@ async function main() { | |||
rt.send({ | |||
type: 'session.update', | |||
session: { | |||
modalities: ['foo'] as any, | |||
modalities: ['text'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops thanks for fixing!
src/beta/realtime/websocket.ts
Outdated
if (azureCheck) { | ||
if (this.url.searchParams.get('Authorization') !== null) { | ||
this.url.searchParams.set('Authorization', '<REDACTED>'); | ||
} else { | ||
this.url.searchParams.set('api-key', '<REDACTED>'); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: why do we need to do this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We try not to expose secrets and this.url
is exposed on the client. Follows the same spirit of #1218
src/beta/realtime/websocket.ts
Outdated
@@ -26,6 +26,7 @@ export class OpenAIRealtimeWebSocket extends OpenAIRealtimeEmitter { | |||
props: { | |||
model: string; | |||
dangerouslyAllowBrowser?: boolean; | |||
onUrl?: (url: URL) => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit
onUrl?: (url: URL) => void; | |
onURL?: (url: URL) => void; |
also would be nice to mention / mark this as internal-only
onUrl?: (url: URL) => void; | |
/** | |
* Callback to mutate the URL, needed for Azure. | |
* @internal | |
*/ | |
onURL?: (url: URL) => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 9a73f6f
src/beta/realtime/websocket.ts
Outdated
this.url = buildRealtimeURL(client, props.model); | ||
props.onUrl?.(this.url); | ||
|
||
const azureCheck = isAzure(client); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional suggestion: I would personally find this easier to read if isAzure(client)
was just inlined instead of extracted to a variable like this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 9a73f6f
src/beta/realtime/internal-base.ts
Outdated
@@ -1,6 +1,7 @@ | |||
import { RealtimeClientEvent, RealtimeServerEvent, ErrorEvent } from '../../resources/beta/realtime/realtime'; | |||
import { EventEmitter } from '../../lib/EventEmitter'; | |||
import { OpenAIError } from '../../error'; | |||
import OpenAI, { AzureOpenAI } from 'openai'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit
import OpenAI, { AzureOpenAI } from 'openai'; | |
import OpenAI, { AzureOpenAI } from '../../index'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 9a73f6f
@@ -509,6 +509,26 @@ const result = await openai.chat.completions.create({ | |||
console.log(result.choices[0]!.message?.content); | |||
``` | |||
|
|||
### Realtime API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @kwhinnery-openai I suspect you'll want to do some wordsmithing here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will follow on with some copy edits, but no blocking feedback
Support Azure Realtime API
In order to do so, this PR adds an azure factory method to
OpenAIRealtimeWebSocket
andOpenAIRealtimeWS
classes that can handle asynchronous operations required during WebSocket connection setup.Key Changes
1. New
azure
Methodazure
method enables asynchronous fetching of the Azure AD token and then pass it to the class constructor.2. New
_getAzureADToken
Method inAzureOpenAI
_getAzureADToken
method in theAzureOpenAI
class to provide refreshed Azure AD tokens.