[OpenAI.Inference] Adding typespec definitions for DALL-E by glecaros · Pull Request #24382 · Azure/azure-rest-api-specs

glecaros · 2023-06-09T18:50:25Z

openapi-workflow-bot · 2023-06-09T18:50:28Z

Hi, @glecaros Thanks for your PR. I am workflow bot for review process. Here are some small tips.

Please ensure to do self-check against checklists in first PR comment.

PR assignee is the person auto-assigned and responsible for your current PR reviewing and merging.

For specs comparison cross API versions, Use API Specs Comparison Report Generator

If there is CI failure(s), to fix CI error(s) is mandatory for PR merging; or you need to provide justification in PR comment for explanation. How to fix?

Any feedback about review process or workflow bot, pls contact swagger and tools team. vscswagger@microsoft.com

openapi-pipeline-app · 2023-06-09T18:50:30Z

Swagger Validation Report

️❌BreakingChange: 10 Errors, 0 Warnings failed [Detail]

compared swaggers (via Oad v0.10.4)]	new version	base version
generated.json	2023-06-01-preview(8a28131)	2023-06-01-preview(main)
generated.json	2022-12-01(8a28131)	2022-12-01(main)
generated.json	2023-05-15(8a28131)	2023-05-15(main)

Rule	Message
`❌` 1038 - AddedPath	The new version is adding a path that was not found in the old version. New: inference/preview/2023-06-01-preview/generated.json#L62:5
`❌` 1038 - AddedPath	The new version is adding a path that was not found in the old version. New: inference/preview/2023-06-01-preview/generated.json#L113:5
`❌` 1038 - AddedPath	The new version is adding a path that was not found in the old version. New: inference/preview/2023-06-01-preview/generated.json#L164:5
`❌` 1038 - AddedPath	The new version is adding a path that was not found in the old version. New: inference/preview/2023-06-01-preview/generated.json#L215:5
`❌` 1038 - AddedPath	The new version is adding a path that was not found in the old version. New: inference/preview/2023-06-01-preview/generated.json#L266:5
`❌` 1038 - AddedPath	The new version is adding a path that was not found in the old version. New: inference/stable/2022-12-01/generated.json#L62:5
`❌` 1038 - AddedPath	The new version is adding a path that was not found in the old version. New: inference/stable/2022-12-01/generated.json#L113:5
`❌` 1038 - AddedPath	The new version is adding a path that was not found in the old version. New: inference/stable/2023-05-15/generated.json#L62:5
`❌` 1038 - AddedPath	The new version is adding a path that was not found in the old version. New: inference/stable/2023-05-15/generated.json#L113:5
`❌` 1038 - AddedPath	The new version is adding a path that was not found in the old version. New: inference/stable/2023-05-15/generated.json#L164:5

️️✔️Breaking Change(Cross-Version) succeeded [Detail] [Expand]

There are no breaking changes.

️️✔️CredScan succeeded [Detail] [Expand]

There is no credential detected.

️⚠️LintDiff: 68 Warnings warning [Detail]

compared tags (via openapi-validator v2.1.3)	new version	base version
release_2022_03_01_preview	release_2022_03_01_preview(8a28131)	default(main)
release_2022_06_01_preview	release_2022_06_01_preview(8a28131)	default(main)
release_2022_12_01	release_2022_12_01(8a28131)	default(main)
release_2022_12_01_autogen	release_2022_12_01_autogen(8a28131)	default(main)
release_2023_03_15_preview	release_2023_03_15_preview(8a28131)	default(main)
release_2023_05_15	release_2023_05_15(8a28131)	default(main)
release_2023_05_15_autogen	release_2023_05_15_autogen(8a28131)	default(main)
release_2023_06_01_preview	release_2023_06_01_preview(8a28131)	default(main)
release_2023_06_01_preview_autogen	release_2023_06_01_preview_autogen(8a28131)	default(main)

[must fix]The following errors/warnings are introduced by current PR:

Only 30 items are listed, please refer to log for more details.

Rule	Message	Related RPC [For API reviewers]
⚠️ SecurityDefinitionDescription	Security definition should have a description. Location: inference/preview/2023-06-01-preview/generated.json#L46
⚠️ SecurityDefinitionDescription	Security definition should have a description. Location: inference/preview/2023-06-01-preview/generated.json#L51
⚠️ OperationId	OperationId should be of the form 'Noun_Verb' Location: inference/preview/2023-06-01-preview/generated.json#L64
⚠️ PathParameterSchema	Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: inference/preview/2023-06-01-preview/generated.json#L70
⚠️ ParameterDescription	Parameter should have a description. Location: inference/preview/2023-06-01-preview/generated.json#L77
⚠️ OperationId	OperationId should be of the form 'Noun_Verb' Location: inference/preview/2023-06-01-preview/generated.json#L115
⚠️ PathParameterSchema	Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: inference/preview/2023-06-01-preview/generated.json#L121
⚠️ ParameterDescription	Parameter should have a description. Location: inference/preview/2023-06-01-preview/generated.json#L128
⚠️ PaginationResponse	Operation might be pageable. Consider adding the x-ms-pageable extension. Location: inference/preview/2023-06-01-preview/generated.json#L165
⚠️ OperationId	OperationId should be of the form 'Noun_Verb' Location: inference/preview/2023-06-01-preview/generated.json#L166
⚠️ PathParameterSchema	Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: inference/preview/2023-06-01-preview/generated.json#L172
⚠️ ParameterDescription	Parameter should have a description. Location: inference/preview/2023-06-01-preview/generated.json#L179
⚠️ OperationId	OperationId should be of the form 'Noun_Verb' Location: inference/preview/2023-06-01-preview/generated.json#L217
⚠️ ParameterDescription	Parameter should have a description. Location: inference/preview/2023-06-01-preview/generated.json#L223
⚠️ OperationId	OperationId should be of the form 'Noun_Verb' Location: inference/preview/2023-06-01-preview/generated.json#L268
⚠️ PathParameterSchema	Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: inference/preview/2023-06-01-preview/generated.json#L274
⚠️ SchemaNamesConvention	Schema name should be Pascal case. Location: inference/preview/2023-06-01-preview/generated.json#L311
⚠️ SchemaNamesConvention	Schema name should be Pascal case. Location: inference/preview/2023-06-01-preview/generated.json#L346
⚠️ SchemaNamesConvention	Schema name should be Pascal case. Location: inference/preview/2023-06-01-preview/generated.json#L359
⚠️ PropertyType	Property should have a defined type. Location: inference/preview/2023-06-01-preview/generated.json#L385
⚠️ EnumInsteadOfBoolean	Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: inference/preview/2023-06-01-preview/generated.json#L503
⚠️ Nullable	Avoid the use of x-nullable. Location: inference/preview/2023-06-01-preview/generated.json#L583
⚠️ PropertyType	Property should have a defined type. Location: inference/preview/2023-06-01-preview/generated.json#L587
⚠️ Nullable	Avoid the use of x-nullable. Location: inference/preview/2023-06-01-preview/generated.json#L651
⚠️ EnumInsteadOfBoolean	Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: inference/preview/2023-06-01-preview/generated.json#L740
⚠️ EnumInsteadOfBoolean	Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: inference/preview/2023-06-01-preview/generated.json#L770
⚠️ SchemaDescriptionOrTitle	Schema should have a description or title. Location: inference/preview/2023-06-01-preview/generated.json#L1076
⚠️ SecurityDefinitionDescription	Security definition should have a description. Location: inference/stable/2022-12-01/generated.json#L46
⚠️ SecurityDefinitionDescription	Security definition should have a description. Location: inference/stable/2022-12-01/generated.json#L51
⚠️ OperationId	OperationId should be of the form 'Noun_Verb' Location: inference/stable/2022-12-01/generated.json#L64

️️✔️Avocado succeeded [Detail] [Expand]

Validation passes for Avocado.

️️✔️ApiReadinessCheck succeeded [Detail] [Expand]

️⚠️~[Staging] ServiceAPIReadinessTest: 0 Warnings warning [Detail]

API Test is not triggered due to precheck failure. Check pipeline log for details.

️️✔️SwaggerAPIView succeeded [Detail] [Expand]

️️✔️CadlAPIView succeeded [Detail] [Expand]

️❌TypeSpecAPIView: 0 Errors, 0 Warnings failed [Detail]

️️✔️ModelValidation succeeded [Detail] [Expand]

Validation passes for ModelValidation.

️️✔️SemanticValidation succeeded [Detail] [Expand]

Validation passes for SemanticValidation.

️️✔️PoliCheck succeeded [Detail] [Expand]

Validation passed for PoliCheck.

️️✔️PrettierCheck succeeded [Detail] [Expand]

Validation passes for PrettierCheck.

️️✔️SpellCheck succeeded [Detail] [Expand]

Validation passes for SpellCheck.

️️✔️Lint(RPaaS) succeeded [Detail] [Expand]

Validation passes for Lint(RPaaS).

️️✔️CadlValidation succeeded [Detail] [Expand]

Validation passes for CadlValidation.

️️✔️TypeSpec Validation succeeded [Detail] [Expand]

Validation passes for TypeSpec Validation.

️️✔️PR Summary succeeded [Detail] [Expand]

Validation passes for Summary.

_{Posted by Swagger Pipeline | How to fix these errors?}

openapi-pipeline-app · 2023-06-09T18:50:31Z

Swagger pipeline restarted successfully, please wait for status update in this comment.

openapi-pipeline-app · 2023-06-09T18:50:32Z

Generated ApiView

Language	Package Name	ApiView Link
TypeSpec	OpenAI.Inference	Create ApiView failed. Please ask PR assignee for help

specification/cognitiveservices/OpenAI.Inference/models/images.tsp

specification/cognitiveservices/OpenAI.Inference/main.tsp

openapi-workflow-bot · 2023-06-13T03:18:52Z

Hi @glecaros, Your PR has some issues. Please fix the CI sequentially by following the order of Avocado, semantic validation, model validation, breaking change, lintDiff. If you have any questions, please post your questions in this channel https://aka.ms/swaggersupport.

Task	How to fix	Priority
Avocado	Fix-Avocado	High
Semantic validation	Fix-SemanticValidation-Error	High
Model validation	Fix-ModelValidation-Error	High
LintDiff	Fix-LintDiff	high

If you need further help, please feedback via swagger feedback.

openapi-workflow-bot · 2023-06-13T03:18:53Z

Hi, @glecaros, For review efficiency consideration, when creating a new api version, it is required to place API specs of the base version in the first commit, and push new version updates into successive commits. You can use OpenAPIHub to initialize the PR for adding a new version. For more details refer to the wiki. Or you could onboard API spec pipeline

azure-pipelines · 2023-06-16T22:48:46Z

Azure Pipelines successfully started running 2 pipeline(s).

mikeharder · 2023-06-16T23:16:20Z

The check for azure-sdk-for-net-track2 actually passed, but status never made it from DevOps to GitHub. This happens sometimes and unfortunately there's not much we can do about it.

The check for azure-sdk-for-java appears to have hung trying to clone a repo from GitHub, I will re-run it.

The check for azure-sdk-for-js did fail, I will investigate this one, but I'm not sure if all of the azure-sdk-for-* checks are even working right now.

The check for azure-sdk-for-net-track2 failed now. Not sure how it passed earlier but it doesn't matter. Current failure is caused by this compiler error:

/mnt/vss/_work/1/s/azure-sdk-for-net/sdk/openai/Azure.AI.OpenAI/src/Custom/StreamingChoice.cs(37,65):
error CS1061: 'CompletionsFinishReason' does not contain a definition for 'Value' and no accessible extension
method 'Value' accepting a first argument of type 'CompletionsFinishReason' could be found (are you
missing a using directive or an assembly reference?)
[/mnt/vss/_work/1/s/azure-sdk-for-net/sdk/openai/
Azure.AI.OpenAI/src/Azure.AI.OpenAI.csproj::TargetFramework=netstandard2.0]

@glecaros: Does this error make sense to you, and do you know how to fix it?

@m-nash: Are you the right person to assist here if needed?

I am personally not very familiar with the language SDK generation side of things yet.

mikeharder · 2023-06-16T23:20:51Z

Check azure-sdk-for-java is failing with this error:

10:57:42.997 cmdout 	[generate.py] [ERROR] /mnt/vss/_work/1/s/azure-sdk-for-java/sdk/openai/
azure-ai-openai/src/main/java/module-info.java:[7,39] module not found: com.azure.core.experimental

@srnagar: Are you the right person to assist with this error?

mikeharder · 2023-06-16T23:23:25Z

Check azure-sdk-for-js failed with this error (but failed to report status to GitHub again):

npx tsp compile /mnt/vss/_work/1/s/azure-sdk-for-js/sdk/openai/azure-ai-openai/TempTypeSpecFiles/OpenAI.Inference/main.tsp --emit @azure-tools/typespec-ts --option @azure-tools/typespec-ts.emitter-output-dir=/mnt/vss/_work/1/s/azure-sdk-for-js/sdk/openai/azure-ai-openai/
TypeSpec compiler v0.45.0
Failed to format file: /mnt/vss/_work/1/s/azure-sdk-for-js/sdk/openai/azure-ai-openai/src/rest/models.ts
ExternalError: Emitter "@azure-tools/typespec-ts" failed!
File issue at https://github.com/Azure/autorest.typescript/issues
Error: Cannot find module './parser-typescript.js'

@deyaaeldeen: Are you the right person to assist with this?

mikeharder · 2023-06-16T23:24:17Z

@raych1: Three of the "sdk" checks are failing for this PR. Are these checks expected to be passing now on valid PRs, so if these checks are failing it means something is wrong in the PR? Or are these checks still not working reliably?

raych1 · 2023-06-21T07:39:24Z

@raych1: Three of the "sdk" checks are failing for this PR. Are these checks expected to be passing now on valid PRs, so if these checks are failing it means something is wrong in the PR? Or are these checks still not working reliably?

@mikeharder , all the emitter are commented out except autorest. That's why SDK CI checks are skipped.

weidongxu-microsoft · 2023-06-21T07:50:04Z

specification/cognitiveservices/OpenAI.Inference/routes.tsp

+#suppress "@azure-tools/typespec-azure-core/no-rpc-path-params" "Allowed because this is a non-standard status polling operation."
+@doc("Returns the status of the images operation")
+@added(ServiceApiVersions.v2023_06_01_Preview)
+@route("/operations/images/{operationId}")
+op getImageOperationStatus is RpcOperation<
+  {
+    @doc(".") @path operationId: string;
+  },
+  ImageOperationResponse
+>;
+
+#suppress "@azure-tools/typespec-azure-core/use-standard-operations" ""
+@doc("Starts the generation of a batch of images from a text caption")
+@added(ServiceApiVersions.v2023_06_01_Preview)
+@route("/images/generations:submit")
+@pollingOperation(
+  getImageOperationStatus,
+  {
+    operationId: ResponseProperty<"id">,
+  }
+)
+op startGenerateImage is OaiLongRunningRpcOperation<
+  ImageGenerationOptions,
+  ImageOperationResponse,
+  ImageOperationStatus
+>;


Do we have a runtime log on the LRO responses? Or at least give us a response of last LRO poll.

Ask because the LRO may not be correctly defined.

It may need to be written as

op startGenerateImage is OaiLongRunningRpcOperation< ImageGenerationOptions, ImageOperationResponse, ImageResponse >;

The TStatusResult in template is the model in "result" property. It is a result in status, not a status.
https://github.com/Azure/typespec-azure/blob/main/packages/typespec-azure-core/lib/foundations.tsp#L37-L51

Or you may want to use Azure.Core.LongRunningRpcOperation

specification/cognitiveservices/OpenAI.Inference/models/images.tsp

mikeharder · 2023-06-21T19:00:39Z

@raych1: Three of the "sdk" checks are failing for this PR. Are these checks expected to be passing now on valid PRs, so if these checks are failing it means something is wrong in the PR? Or are these checks still not working reliably?

@mikeharder , all the emitter are commented out except autorest. That's why SDK CI checks are skipped.

The emitters were commented out because the CI checks were failing earlier. We decided to merge this PR without the language emitters for now, and we will try to re-enable the language emitters in subsequent PRs, where we will need to investigate any failures.

specification/cognitiveservices/OpenAI.Inference/models/images.tsp

* Adding typespec definitions for DALL-E * feedback * Updating api version. * size enum * update * adding missing error * Adding open api. * fixing CI * fixing 'created' type. * generated files * updating examples * fixes * proper target * adding missing examples * fixes * fixing examples * more example updates * more fixes * regenerating files * more fixes * adding temporary fix * Link suppress of "union-unsupported" to tracking issue * tsp format * feedback * enum values * fixing example * renaming ImageResult to ImageLocation * rename * Adding readme * Adding extra files * feedback + linter fixes * generated files * cleanup * fixes * fix * Update specification/cognitiveservices/OpenAI.Inference/routes.tsp Co-authored-by: Johan Stenberg (MSFT) <johan.stenberg@microsoft.com> * feedback * fixes * fixing param name * adding deps * package dir * disabling failing emitters * reenabling csharp * indenting * testing java & ts * disabling back --------- Co-authored-by: Mike Harder <mharder@microsoft.com> Co-authored-by: Johan Stenberg (MSFT) <johan.stenberg@microsoft.com>

Adding typespec definitions for DALL-E

cce8dfc

glecaros requested a review from yangyuan as a code owner June 9, 2023 18:50

openapi-assignment-bot bot assigned jhendrixMSFT Jun 9, 2023

ghost added the Cognitive Services label Jun 9, 2023