-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Storage] Make the CORS rules nullable #7766
Conversation
…nto CORS_Nullable
- from: swagger-document | ||
where: $.definitions.BlobServiceProperties | ||
transform: > | ||
$.properties.Cors["x-az-nullable-array"] = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a note about this new property at the top of https://github.com/Azure/azure-sdk-for-net/blob/master/sdk/storage/Azure.Storage.Common/swagger/Generator/readme.md
I'm trying to keep track of all the extensions we've added.
@@ -151,7 +151,8 @@ export interface IProperty { | |||
required?: boolean, | |||
readonly: boolean, | |||
xml?: IXmlSettings, | |||
model: IModelType | |||
model: IModelType, | |||
isNullable?: boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be isNullable?
or can it just be isNullable
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really wanted to not have this as an optional parameter but since TypeScript doesn't assign default values for primitive types, I had to make it as optional (microsoft/TypeScript#5113)
If I'd make isNullable
required then we would have to set isNullable
when we add headers and body in createResponseType() method.
We can set false
in those cases but do we need to set isNullable
when creating a response type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
In our (Blob/Queue/File)ServiceClient.SetPropertiesAsync method, allow the list of Cors rules to be null to indicate they should be ignored instead of cleared.
In order to do that, created a new swagger attribute
"x-az-nullable-array"
that we specify on these models and then allow null to prevent them from showing up in the generated code