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

.Net: DefaultValue for OpenAPI payload properties #4612

Merged

Conversation

SergeyMenshykh
Copy link
Member

Motivation and Context

Currently, the default values of parameters in OpenAPI plugins have the 'string?' type, even though their original type specified in the OpenAPI document schema might be different. A dedicated type conversion functionality exists to convert from the original type to a string. However, both the string type for the parameters/arguments and the type conversion logic are no longer necessary, as SK has moved away from string-type arguments and recently added support for .NET primitive types and complex types.

Furthermore, the RestApiOperationPayloadProperty class does not have a property for default values, so a default value specified in an OpenAPI document is lost and cannot be accessed from the code.

Description

  1. The type of the DefaultValue property in the RestApiOperationParameter class has been changed from string? to object? to better represent the actual type of the parameter's default value and eliminate unnecessary conversion to string.

  2. A new DefaultValue property has been added to the RestApiOperationPayloadProperty, making it consistent with the RestApiOperationParameter class.

…eter class has been changed from string? to object? to better represent the actual type of the parameter's default value.

2. The DefaultType property has been added to the RestApiOperationPayloadProperty, making it consistent with the RestApiOperationParameter class.
@SergeyMenshykh SergeyMenshykh added the PR: ready for review All feedback addressed, ready for reviews label Jan 17, 2024
@SergeyMenshykh SergeyMenshykh linked an issue Jan 18, 2024 that may be closed by this pull request
@SergeyMenshykh SergeyMenshykh added this pull request to the merge queue Jan 19, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 19, 2024
@SergeyMenshykh SergeyMenshykh added this pull request to the merge queue Jan 19, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 19, 2024
@SergeyMenshykh SergeyMenshykh added this pull request to the merge queue Jan 19, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 19, 2024
@SergeyMenshykh SergeyMenshykh added this pull request to the merge queue Jan 19, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 19, 2024
@SergeyMenshykh SergeyMenshykh added this pull request to the merge queue Jan 19, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 19, 2024
@SergeyMenshykh SergeyMenshykh added this pull request to the merge queue Jan 19, 2024
Merged via the queue into microsoft:main with commit a5bc63d Jan 19, 2024
18 checks passed
@SergeyMenshykh SergeyMenshykh deleted the default-value-for-payload-props branch January 19, 2024 15:09
Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

LGTM

Bryan-Roe pushed a commit to Bryan-Roe/semantic-kernel that referenced this pull request Oct 6, 2024
### Motivation and Context  
Currently, the default values of parameters in OpenAPI plugins have the
'string?' type, even though their original type specified in the OpenAPI
document schema might be different. A dedicated type conversion
functionality exists to convert from the original type to a string.
However, both the string type for the parameters/arguments and the type
conversion logic are no longer necessary, as SK has moved away from
string-type arguments and recently added support for .NET primitive
types and complex types.
   
Furthermore, the `RestApiOperationPayloadProperty` class does not have a
property for default values, so a default value specified in an OpenAPI
document is lost and cannot be accessed from the code.
   
### Description  
1. The type of the `DefaultValue` property in the
`RestApiOperationParameter` class has been changed from string? to
object? to better represent the actual type of the parameter's default
value and eliminate unnecessary conversion to string.
   
2. A new `DefaultValue` property has been added to the
`RestApiOperationPayloadProperty`, making it consistent with the
RestApiOperationParameter class.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.NET Issue or Pull requests regarding .NET code PR: ready for review All feedback addressed, ready for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.Net: Investigate OpenAPI default value requirement for BC
5 participants