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

chore: Fix resource group header #185

Merged
merged 10 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions packages/ai-api/src/utils/deployment-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@ export type ModelDeployment<ModelNameT = string> =
| ModelNameT
| ((ModelConfig<ModelNameT> | DeploymentIdConfig) & ResourceGroupConfig);

/**
* @internal
*/
export function getResourceGroup(
modelDeployment: ModelDeployment
): string | undefined {
return typeof modelDeployment === 'object'
? modelDeployment.resourceGroup
: undefined;
MatKuhr marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Type guard to check if the given deployment configuration is a deployment ID configuration.
* @param modelDeployment - Configuration to check.
Expand Down
27 changes: 27 additions & 0 deletions packages/core/src/http-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,31 @@ describe('http-client', () => {
expect(res.status).toBe(200);
expect(res.data).toEqual(mockPromptResponse);
}, 10000);

it('should execute a request to the AI Core service with a custom resource gorup', async () => {
tomfrenken marked this conversation as resolved.
Show resolved Hide resolved
const mockPrompt = { prompt: 'some test prompt' };
const mockPromptResponse = { completion: 'some test completion' };

const scope = nock(aiCoreDestination.url, {
reqheaders: {
'ai-resource-group': 'custom-resource-group',
'ai-client-type': 'AI SDK JavaScript'
}
})
.post('/v2/some/endpoint', mockPrompt)
.query({ 'api-version': 'mock-api-version' })
.reply(200, mockPromptResponse);

const res = await executeRequest(
{
url: '/some/endpoint',
apiVersion: 'mock-api-version',
resourceGroup: 'custom-resource-group'
},
mockPrompt
);
expect(scope.isDone()).toBe(true);
expect(res.status).toBe(200);
expect(res.data).toEqual(mockPromptResponse);
});
});
12 changes: 8 additions & 4 deletions packages/core/src/http-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,14 @@ export interface EndpointOptions {
* The specific endpoint to call.
*/
url: string;

/**
* The API version to use.
*/
apiVersion?: string;
ZhongpinWang marked this conversation as resolved.
Show resolved Hide resolved
/**
* The resource group to use.
*/
resourceGroup?: string;
}
/**
* Executes a request to the AI Core service.
Expand All @@ -49,10 +52,10 @@ export async function executeRequest(
requestConfig?: CustomRequestConfig
): Promise<HttpResponse> {
const aiCoreDestination = await getAiCoreDestination();
const { url, apiVersion } = endpointOptions;
const { url, apiVersion, resourceGroup } = endpointOptions;

const mergedRequestConfig = {
...mergeWithDefaultRequestConfig(apiVersion, requestConfig),
...mergeWithDefaultRequestConfig(apiVersion, resourceGroup, requestConfig),
data: JSON.stringify(data)
};

Expand All @@ -69,13 +72,14 @@ export async function executeRequest(

function mergeWithDefaultRequestConfig(
apiVersion?: string,
resourceGroup?: string,
requestConfig?: CustomRequestConfig
): HttpRequestConfig {
const defaultConfig: HttpRequestConfig = {
method: 'post',
headers: {
'content-type': 'application/json',
'ai-resource-group': 'default',
'ai-resource-group': resourceGroup ?? 'default',
'ai-client-type': 'AI SDK JavaScript'
},
params: apiVersion ? { 'api-version': apiVersion } : {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,93 @@ describe('Azure OpenAI chat client', () => {

await expect(client.run(prompt)).rejects.toThrow('status code 400');
});

it('executes a request with the custom request group', async () => {
tomfrenken marked this conversation as resolved.
Show resolved Hide resolved
tomfrenken marked this conversation as resolved.
Show resolved Hide resolved
const customChatCompletionEndpoint = {
url: 'inference/deployments/1234/chat/completions',
apiVersion,
resourceGroup: 'custom-resource-group'
};

const mockResponse =
parseMockResponse<AzureOpenAiCreateChatCompletionResponse>(
'foundation-models',
'azure-openai-chat-completion-success-response.json'
);

const prompt = {
messages: [
{
role: 'user' as const,
content: 'Where is the deepest place on earth located'
}
]
};

mockInference(
{
data: prompt
},
{
data: mockResponse,
status: 200
},
customChatCompletionEndpoint
);

const clientWithResourceGroup = new AzureOpenAiChatClient({
deploymentId: '1234',
resourceGroup: 'custom-resource-group'
});

const response = await clientWithResourceGroup.run(prompt);
expect(response.data).toEqual(mockResponse);
});

it('it fails when the wrong resource group is set', async () => {
tomfrenken marked this conversation as resolved.
Show resolved Hide resolved
const customChatCompletionEndpoint = {
url: 'inference/deployments/1234/chat/completions',
apiVersion,
resourceGroup: 'custom-resource-group'
};

const prompt = {
messages: [
{
role: 'user' as const,
content: 'Where is the deepest place on earth located'
}
]
};

const mockResponse =
parseMockResponse<AzureOpenAiCreateChatCompletionResponse>(
'foundation-models',
'azure-openai-chat-completion-success-response.json'
);

mockInference(
tomfrenken marked this conversation as resolved.
Show resolved Hide resolved
{
data: prompt
},
{
data: mockResponse,
status: 200
},
customChatCompletionEndpoint
);

mockInference(
{
data: prompt
},
{
data: null,
status: 404
},
chatCompletionEndpoint
);

expect(client.run(prompt)).rejects.toThrow();
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { type CustomRequestConfig, executeRequest } from '@sap-ai-sdk/core';
import {
getDeploymentId,
getResourceGroup,
type ModelDeployment
} from '@sap-ai-sdk/ai-api/internal.js';
import type { AzureOpenAiCreateChatCompletionRequest } from './client/inference/schema/index.js';
Expand Down Expand Up @@ -31,10 +32,12 @@ export class AzureOpenAiChatClient {
this.modelDeployment,
'azure-openai'
);
const resourceGroup = getResourceGroup(this.modelDeployment);
const response = await executeRequest(
{
url: `/inference/deployments/${deploymentId}/chat/completions`,
apiVersion
apiVersion,
resourceGroup
},
data,
requestConfig
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { type CustomRequestConfig, executeRequest } from '@sap-ai-sdk/core';
import {
getDeploymentId,
getResourceGroup,
type ModelDeployment
} from '@sap-ai-sdk/ai-api/internal.js';
import { AzureOpenAiEmbeddingResponse } from './azure-openai-embedding-response.js';
Expand Down Expand Up @@ -34,8 +35,13 @@ export class AzureOpenAiEmbeddingClient {
this.modelDeployment,
'azure-openai'
);
const resourceGroup = getResourceGroup(this.modelDeployment);
const response = await executeRequest(
{ url: `/inference/deployments/${deploymentId}/embeddings`, apiVersion },
{
url: `/inference/deployments/${deploymentId}/embeddings`,
apiVersion,
resourceGroup
},
data,
requestConfig
);
Expand Down
3 changes: 2 additions & 1 deletion packages/orchestration/src/orchestration-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ export class OrchestrationClient {

const response = await executeRequest(
{
url: `/inference/deployments/${deploymentId}/completion`
url: `/inference/deployments/${deploymentId}/completion`,
resourceGroup: this.deploymentConfig?.resourceGroup
},
body,
requestConfig
Expand Down
4 changes: 2 additions & 2 deletions test-util/mock-http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ export function mockInference(
},
endpoint: EndpointOptions = mockEndpoint
): nock.Scope {
const { url, apiVersion } = endpoint;
const { url, apiVersion, resourceGroup } = endpoint;
const destination = getMockedAiCoreDestination();
return nock(destination.url, {
reqheaders: {
'ai-resource-group': 'default',
'ai-resource-group': resourceGroup ?? 'default',
authorization: `Bearer ${destination.authTokens?.[0].value}`
}
})
Expand Down