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: Remove deprecated models #607

Merged
merged 8 commits into from
Mar 26, 2025
Merged

chore: Remove deprecated models #607

merged 8 commits into from
Mar 26, 2025

Conversation

shibeshduw
Copy link
Contributor

Context

Closes SAP/ai-sdk-js-backlog#257.

What this PR does and why it is needed

Replaces text-embedding-ada-002 in our sample-code and documentation with the suggested replacements according to SAP notes.

@shibeshduw shibeshduw marked this pull request as ready for review March 21, 2025 08:45
Copy link
Contributor

@ZhongpinWang ZhongpinWang left a comment

Choose a reason for hiding this comment

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

Check my general question below. Removing text-embedding-ada-002 looks good.

@@ -18,7 +18,7 @@ export type AzureOpenAiChatModel = LiteralUnion<
* Azure OpenAI models for embedding.
*/
export type AzureOpenAiEmbeddingModel = LiteralUnion<
'text-embedding-ada-002' | 'text-embedding-3-small' | 'text-embedding-3-large'
'text-embedding-3-small' | 'text-embedding-3-large'
Copy link
Contributor

Choose a reason for hiding this comment

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

[q] A general question. Should we start removing deprecated (not retired) models already from types? If so, should we already remove other deprecated models? I would prefer we remove them now to give user some buffer to replace them before it gets permanently removed from remote service. @jjtang1985 What is your opinion?

Copy link
Contributor

@ZhongpinWang ZhongpinWang left a comment

Choose a reason for hiding this comment

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

LGTM

@shibeshduw shibeshduw merged commit 627a152 into main Mar 26, 2025
10 checks passed
@shibeshduw shibeshduw deleted the replace-deprecated-models branch March 26, 2025 09:28
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.

2 participants