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: applicationinsights delete examples #6636

Merged

Conversation

nschonni
Copy link
Contributor

Empty schema object was passing the empty string body in examples.

Empty schema object was passing the empty string body in examples.
@AutorestCI
Copy link

AutorestCI commented Jul 16, 2019

Automation for azure-sdk-for-python

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-python#6521

@AutorestCI
Copy link

AutorestCI commented Jul 16, 2019

Automation for azure-sdk-for-go

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-go#5360

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Jul 16, 2019

Automation for azure-sdk-for-java

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-java#3266

@nschonni
Copy link
Contributor Author

Not sure if this is OK because it does change the SDK generation

@yungezz
Copy link
Member

yungezz commented Jul 16, 2019

it's better we can get it validated via Swagger to SDK automation v2

@nschonni
Copy link
Contributor Author

nschonni commented Jul 16, 2019

@yungezz sorry, I don't understand. Is that an internal MS tool that you want to run or you're saying this is better because it validates now? I just got the error from OAV when I was playing around.

@yungezz
Copy link
Member

yungezz commented Jul 16, 2019

what OAV error you met? Yes I'm talking some internal tool which will run SDK autogeneration.

@nschonni
Copy link
Contributor Author

Ah, sorry, in this case I think I searched for the empty schema value, because it hides the OAV error around schema missmatch

$ oav validate-example specification/applicationinsights/resource-manager/Microsoft.Insights/stable/2015-05-01/compon 
entAnnotations_API.json -p
Validating "examples" and "x-ms-examples" in  specification/applicationinsights/resource-manager/Microsoft.Insights/stable/2015-05-01/componentAnnotations_API.json:

 error : 
operationId: Annotations_Delete
scenario: AnnotationsDelete
source: response
responseCode: '200'
severity: 0
code: RESPONSE_SCHEMA_NOT_IN_SPEC
details:
  code: RESPONSE_SCHEMA_NOT_IN_SPEC
  id: OAV113
  message: >-
    Response statusCode "200" for operation "Annotations_Delete" has response
    body provided in the example, however the response does not have a "schema"
    defined in the swagger spec.
  position:
    line: 135
    column: 17
  url: >-
    specification/applicationinsights/resource-manager/Microsoft.Insights/stable/2015-05-01/componentAnnotations_API.json
  title: >-
    #/paths/~1subscriptions~1{subscriptionId}~1resourceGroups~1{resourceGroupName}~1providers~1Microsoft.Insights~1components~1{resourceName}~1Annotations~1{annotationId}/delete
  directives:
    PutRequestResponseScheme: .*
    ListInOperationName: .*
    PutInOperationName: .*
    XmsResourceInPutResponse: .*
    RequiredPropertiesMissingInResourceModel: .*
    BodyTopLevelProperties: .*
    EnumInsteadOfBoolean: .*
    DefinitionsPropertiesNamesCamelCase: .*
    R2066: .*
  level: error

I'll go file something to see if OAV should also be flagging the empty schema object

@yungezz
Copy link
Member

yungezz commented Jul 26, 2019

from the above error seems OAV complaining about no schema. I don't think this PR should be accepted

@nschonni
Copy link
Contributor Author

No problem, maybe the oav/avocado folks can weigh in because I could be wrong (a few similar PRs are merged) @sergey-shandar @raych1 @ruowan

@raych1
Copy link
Member

raych1 commented Jul 29, 2019

@yungezz , recently OAV has a change to flag on mismatch issue between swagger response body and example response body.
This PR @nschonni made is to correct the use of shema in response swagger definition. i.e. for delete operation, if response doesn't have body then it remove schema as undefined and same in example where shouldn't have body defined.

@yungezz
Copy link
Member

yungezz commented Jul 29, 2019

got it. thansk @raych1 for clarification. Is this PR ok to merge from OAV perspective then?

@raych1
Copy link
Member

raych1 commented Jul 29, 2019

@yungezz , yes it's ok to merge from OAV perspective.

@yungezz yungezz merged commit 8f0d54f into Azure:master Jul 29, 2019
@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 29, 2019

SDK Automation [Logs] (Generated from 35a0224, Iteration 1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants