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

feat: Update Azure OpenAI spec version to 2024-10-21 #312

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ZhongpinWang
Copy link
Contributor

Context

AI/gen-ai-hub-sdk-js-backlog#174.

Current progress:

  • apiVersion updated
  • ✅ client generated with json
  • stream chunk type applied
  • ❌ langchain breaks with missing types @tomfrenken
  • ...

Definition of Done

  • Code is tested (Unit, E2E)
  • Error handling created / updated & covered by the tests above
  • Documentation updated
  • (Optional) Aligned changes with the Java SDK
  • (Optional) Release notes updated

@@ -0,0 +1,15 @@
{
Copy link
Contributor

@deekshas8 deekshas8 Dec 4, 2024

Choose a reason for hiding this comment

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

[q] Do we really need these example files checked in, considering we don't do anything (either documentation/validation) with them? We had previously commented these out, until this extension is handled in the sdk generator

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With yaml, it works without examples folder, but with json, generation will fail and complaining there is no examples folder. Not sure if this is wanted behavior though.

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't work with yaml either, this was the reason we decided to comment x-ms-examples out in the spec.

The SDK generator does not really support this Autorest vendor extension as per my knowledge (for tsdoc purposes, or any other), but breaks if the example file is not there (ideally, this should be handled in the generator to not fail if file is missing).

In the azure spec repository, the only place these example files are used (that I could find) is to run model validation using the oav tool.

IMO, commenting this part out rather than checking in these unused files would be a preferable way to go for now and later fixing it in https://github.com/SAP/cloud-sdk-backlog/issues/1237. @jjtang1985

@MatKuhr How do you handle this in the Java SDK?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One possibility is to use jq (or with npm, node-jq as dev dependency) to remove all x-ms-examples.

I tried to add the following command to foundation-models/pakcage.json to pre-process the spec:

    "pregenerate:azure-openai": "mv ./src/azure-openai/spec/inference.json ./src/azure-openai/spec/inference.json.bak && pnpm node-jq 'walk(if type == \"object\" then (if has(\"x-ms-examples\") then del(.[\"x-ms-examples\"]) else . end) else . end)' src/azure-openai/spec/inference.json.bak > src/azure-openai/spec/inference.json && rm ./src/azure-openai/spec/inference.json.bak",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the package json and removed examples.

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.

3 participants