multipart api design (DO NOT MERGE) #987
Conversation
microsoft-github-policy-service
bot
added
the
lib:tcgc
|
No changes needing a change description found. |
|
You can try these changes at https://cadlplayground.z22.web.core.windows.net/typespec-azure/prs/987/ Check the website changes at https://tspwebsitepr.z22.web.core.windows.net/typespec-azure/prs/987/ |
packages/typespec-client-generator-core/multipart_api_design.md
Outdated
Show resolved
Hide resolved
packages/typespec-client-generator-core/multipart_api_design.md
Outdated
Show resolved
Hide resolved
packages/typespec-client-generator-core/multipart_api_design.md
Outdated
Show resolved
Hide resolved
|
I don't have major issue with the design. |
packages/typespec-client-generator-core/multipart_api_design.md
Outdated
Show resolved
Hide resolved
| 1. Model format | ||
|
|
||
| ``` | ||
| op upload( |
There was a problem hiding this comment.
Will We not support previous define with @body as following?
op upload(@header `content-type`: "mulitpart/form-data",
@body body: {
fileName: string,
headShots: bytes[]
}
There was a problem hiding this comment.
For now typespec still supports this format, so TCGC should still support it I think. After all language emitters support httpPart, we may be able to consider whether disable this format.
There was a problem hiding this comment.
httpPart is for advanced scenario, we are not planning to remove the @body format. Emitter should not care though, it should be transporte for them, they just ask TCGC for the multipart information
| isFilePart: boolean; // whether this part is for file | ||
| multi: boolean; // whether this part is multi in request payload | ||
| headers: HeaderProperty[]; // relates to custom header | ||
| filename?: SdkModelPropertyTypeBase; |
There was a problem hiding this comment.
filename and contentType is meta-data of file property, Itself is not model property. So maybe we shall not define it as SdkModelPropertyTypeBase?
There was a problem hiding this comment.
This is TCGC interface which has wrapped original http type and I will make sure it contain all info that language emitters may need.
|
|
||
| ```typescript | ||
| exprot interface multipartOptionsType { | ||
| isNameDefined: boolean; // whether name is defined in Typespec. For multipart/mixed, name may not be defined for some parts |
There was a problem hiding this comment.
Why not a string field that may be undefined ?
There was a problem hiding this comment.
SdkModelPropertyTypeBase already has required property name. If isNamedDefined is true, emitter shall use name; if false, it means typespec doesn't define name and emitter shall not use the name.
NIT: in Typescript, it is impossible to override the property "name" of base interface to "optional"
tadelesh
removed
the
lib:tcgc
microsoft-github-policy-service
bot
added
the
lib:tcgc
- fixes part of #960 - Pending on microsoft/typespec#3676 Context: Typespec supports 3 kinds of format to define multipart: 1. common ``` op upload( @Header `content-type`: "multipart/form-data", @Body body: { basic: string, headShots: bytes[], } ): void; ``` 2. advanced model format ``` op upload( @Header `content-type`: "multipart/form-data", @multipartBody body: { fullName: HttpPart<string>, headShots: HttpPart<bytes[]> } ): void; ``` 3. advanced array format ``` op upload( @Header `content-type`: "multipart/form-data", @multipartBody body: [ // single HttpPart<string, #{ name: "fullName" }>, HttpPart<bytes[], #{ name: "headShots" }>, ] ): void; ``` - This PR is implementation for API design #987, mainly for 1 and 2 format. Format 3 will be implemented in another PR.
|
You can try these changes here
|
based on microsoft/typespec#3046