Skip to content

Comments

Add example with nested segmentation, fix number types, remove extraneous produces#4311

Merged
annatisch merged 7 commits intoAzure:masterfrom
alexeldeib:aiexample
Oct 31, 2018
Merged

Add example with nested segmentation, fix number types, remove extraneous produces#4311
annatisch merged 7 commits intoAzure:masterfrom
alexeldeib:aiexample

Conversation

@alexeldeib
Copy link
Contributor

@alexeldeib alexeldeib commented Oct 23, 2018

This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.

PR information

  • The title of the PR is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
  • Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
  • If applicable, the PR references the bug/issue that it fixes.
  • Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

@AutorestCI
Copy link

AutorestCI commented Oct 23, 2018

Automation for azure-sdk-for-go

A PR has been created for you:
Azure/azure-sdk-for-go#3181

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Oct 23, 2018

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

@AutorestCI
Copy link

AutorestCI commented Oct 23, 2018

Automation for azure-sdk-for-node

A PR has been created for you:
Azure/azure-sdk-for-node#3937

@AutorestCI
Copy link

AutorestCI commented Oct 23, 2018

Automation for azure-sdk-for-python

A PR has been created for you:
Azure/azure-sdk-for-python#3675

@AutorestCI
Copy link

AutorestCI commented Oct 23, 2018

Automation for azure-sdk-for-js

A PR has been created for you:
Azure/azure-sdk-for-js#292

@annatisch
Copy link
Member

Thanks @alexeldeib - are you able to fix the two failing CI passes?
The Go SDK generation is still failing and there's a number of model validation errors:
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/445374245

Thanks!

@alexeldeib
Copy link
Contributor Author

alexeldeib commented Oct 25, 2018

@annatisch Fixed csv formatting for the example, but I have questions about the other issues.

  1. Go SDK. I see: FATAL: System.InvalidOperationException: codegen for preview swagger microsoft.insights/preview/v1/AppInsights.json must be under a preview subdirectory, but the file is under a preview subdirectory? Any insight here?

  2. Model validation is failing with an error that this property doesn't exist, but it's in the swagger? Possibly due to @ in key name?

"message": 
  "Reference could not be resolved: 
    #/definitions/generated.nested.definitions.eventsResult.properties.%40ai.messages",
    "path" : [
      "definitions","eventsResult","properties","@ai.messages","$ref"],
      "error": "JSON Pointer points to missing location: 
      #/definitions/generated.nested.definitions.eventsResult.properties.%40ai.messages"
    ]

@alexeldeib
Copy link
Contributor Author

Still addressing a model validation issue with additionalProperties.

@alexeldeib
Copy link
Contributor Author

alexeldeib commented Oct 25, 2018

Also have a few unchanged XML request/responses which are now failing CI.

The XML is stuffed as a giant string off the response body -- I think when we wrote these examples, this was the only way to incorporate an XML response that worked. Is there a better way to do this?

@alexeldeib
Copy link
Contributor Author

@annatisch thoughts on above?

@annatisch
Copy link
Member

Hi @marstr - Do you know what would cause this error in the Go generator?

codegen for preview swagger microsoft.insights/preview/v1/AppInsights.json must be under a preview subdirectory

@annatisch
Copy link
Member

Hi @lmazuel - do you know if there have been changes to how XML data is represented in Swagger examples? Samples like this one are now failing:
https://github.com/Azure/azure-rest-api-specs/blob/master/specification/applicationinsights/data-plane/microsoft.insights/preview/v1/examples/events-get-metadata.json

Error:

operationId: Events_GetOdataMetadata
scenario: eventMetadata
source: response
responseCode: '200'
errorDetails:
  code: INVALID_TYPE
  params:
    - object
    - string
  message: Expected type object but found type string

@marstr
Copy link
Member

marstr commented Oct 30, 2018

@annatisch says:
Hi @marstr - Do you know what would cause this error in the Go generator?

codegen for preview swagger microsoft.insights/preview/v1/AppInsights.json must be under a preview subdirectory

Ah yeah, the output path configured for the Azure SDK for Go must be in the "services/preview" folder because this spec is in the preview folder in the specs repository.

@annatisch
Copy link
Member

Thanks @marstr!

Additionally I've confirmed with @sergey-shandar that the model validation errors with regards to XML can be ignored for now.

@alexeldeib
Copy link
Contributor Author

alexeldeib commented Oct 30, 2018

@marstr I think this is due to #3084 creating the preview/stable folders. When the spec was originally introduced, there were no folders to differentiate stable/preview. What's the preferred solution for you? I can update the path here but you'll end up with a redundant version of the SDK in azure-sdk-for-go.

@annatisch thanks for clarifying. I think I covered the other issues with examples, let me know if I missed anything.

e: did fix the Go configuration, so waiting for that to finish.

@AutorestCI
Copy link

AutorestCI commented Oct 30, 2018

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@annatisch annatisch merged commit 29b57c9 into Azure:master Oct 31, 2018
mccleanp pushed a commit that referenced this pull request Mar 23, 2022
* [Nginx] Add certificate API

* Fix

* Fix 2

* Fix 3
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