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

Fix headers and schema definition #27180

Merged
merged 1 commit into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
},
"schema": {
"type": "object",
"x-ms-format": "dfe-object",
"x-ms-format": "dfe-list-generic",
"x-ms-format-element-type": "DatasetSchemaDataElement",
"description": "Columns that define the physical type schema of the dataset. Type: array (or Expression with resultType array), itemType: DatasetSchemaDataElement."
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7007,8 +7007,11 @@
},
"headers": {
"type": "object",
"x-ms-format": "dfe-key-value-pairs",
"description": "Represents the headers that will be sent to the request. For example, to set the language and type on a request: \"headers\" : { \"Accept-Language\": \"en-us\", \"Content-Type\": \"application/json\" }. Type: dictionary (or Expression with resultType dictionary)."
"description": "Represents the headers that will be sent to the request. For example, to set the language and type on a request: \"headers\" : { \"Accept-Language\": \"en-us\", \"Content-Type\": \"application/json\" }. Type: string (or Expression with resultType string).",
"additionalProperties": {
Copy link
Member

@mentat9 mentat9 Jan 5, 2024

Choose a reason for hiding this comment

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

additionalProperties

Use of additionalProperties is not allowed for properties owned by the service. The only time its ok to use it is when the properties are pass thru (user defined) and not subject to any validations. Please schematize this object, make it an array or provide an explanation for why you need to use this pattern. #Resolved

"type": "string",
"x-ms-format": "dfe-string"
}
},
"body": {
"type": "object",
Expand Down Expand Up @@ -8045,8 +8048,11 @@
},
"headers": {
"type": "object",
"x-ms-format": "dfe-key-value-pairs",
"description": "Represents the headers that will be sent to the request. For example, to set the language and type on a request: \"headers\" : { \"Accept-Language\": \"en-us\", \"Content-Type\": \"application/json\" }. Type: dictionary (or Expression with resultType dictionary)."
"description": "Represents the headers that will be sent to the request. For example, to set the language and type on a request: \"headers\" : { \"Accept-Language\": \"en-us\", \"Content-Type\": \"application/json\" }. Type: string (or Expression with resultType string).",
"additionalProperties": {
Copy link
Member

@mentat9 mentat9 Jan 5, 2024

Choose a reason for hiding this comment

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

additionalProperties

Use of additionalProperties is not allowed for properties owned by the service. The only time its ok to use it is when the properties are pass thru (user defined) and not subject to any validations. Please schematize this object, make it an array or provide an explanation for why you need to use this pattern. #Resolved

Copy link
Member

@ruowan ruowan Jan 7, 2024

Choose a reason for hiding this comment

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

The additionalProperties we've included serve as HTTP headers specified by the customer. This schema does not follow a specific pattern.

Moreover, This schemaChange also fix ADF .NET SDK issues. The ADF .NET SDK has some special x-ms-format, identified by a 'dfe-' prefix, such as 'dfe-key-value-pairs'. This update in the schema also addresses an issue in the .NET Track2 SDK, where customers were unable to create web activities with headers (refer to Azure SDK for .NET Issue #39187 for more details).

Copy link
Member

Choose a reason for hiding this comment

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

Are the HTTP headers just passed straight through without any examination or validation applied?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. The HTTP headers just passed through. No validation.

"type": "string",
"x-ms-format": "dfe-string"
Copy link
Member

Choose a reason for hiding this comment

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

If you use x-ms-format, then its type must be object.

Copy link
Member

@ruowan ruowan Dec 26, 2023

Choose a reason for hiding this comment

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

According to our test, if we define the type as object inside the additionalProperties. For additionalProperties the generated model is public IDictionary<string, BinaryData> Headers { get; } and it's not our expected.

}
},
"body": {
"type": "object",
Expand Down Expand Up @@ -8111,8 +8117,11 @@
},
"headers": {
"type": "object",
"x-ms-format": "dfe-key-value-pairs",
"description": "Represents the headers that will be sent to the request. For example, to set the language and type on a request: \"headers\" : { \"Accept-Language\": \"en-us\", \"Content-Type\": \"application/json\" }. Type: dictionary (or Expression with resultType dictionary)."
"description": "Represents the headers that will be sent to the request. For example, to set the language and type on a request: \"headers\" : { \"Accept-Language\": \"en-us\", \"Content-Type\": \"application/json\" }. Type: string (or Expression with resultType string).",
"additionalProperties": {
Copy link
Member

@mentat9 mentat9 Jan 5, 2024

Choose a reason for hiding this comment

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

additionalProperties

Same here. #Resolved

"type": "string",
"x-ms-format": "dfe-string"
Copy link
Member

Choose a reason for hiding this comment

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

Same here

}
},
"body": {
"type": "object",
Expand Down