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

[Blueprints] Add a new example file for using user-assigned MSI #6297

Merged
merged 4 commits into from
Jun 20, 2019
Merged

[Blueprints] Add a new example file for using user-assigned MSI #6297

merged 4 commits into from
Jun 20, 2019

Conversation

filizt
Copy link
Contributor

@filizt filizt commented Jun 12, 2019

Latest improvements:

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

Contribution checklist:

  • I have reviewed the documentation for the workflow.
  • Validation tools were run on swagger spec(s) and have all been fixed in this PR.
  • The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

  • Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
  • adding/removing APIs.
  • adding/removing properties.
  • adding/removing API-version.
  • adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

  • If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
    Please follow the link to find more details on API review process.

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jun 12, 2019

SDK Automation [Logs] (Generated from 4a0ddd1)

Pending Go: Azure/azure-sdk-for-go
  • Package generation pending.
Pending JavaScript: Azure/azure-sdk-for-js
  • Package generation pending.
Pending Python: Azure/azure-sdk-for-python
  • Package generation pending.

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Jun 12, 2019

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

@AutorestCI
Copy link

AutorestCI commented Jun 12, 2019

Automation for azure-sdk-for-python

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

@AutorestCI
Copy link

AutorestCI commented Jun 12, 2019

Automation for azure-sdk-for-js

Nothing to generate for azure-sdk-for-js

@AutorestCI
Copy link

AutorestCI commented Jun 12, 2019

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@AutorestCI
Copy link

AutorestCI commented Jun 12, 2019

Automation for azure-sdk-for-go

Encountered a Subprocess error: (azure-sdk-for-go)

Command: dep ensure
Finished with return code 1
and output:

grouped write of manifest, lock and vendor: error while writing out vendor tree: failed to write dep tree: failed to export golang.org/x/sync: failed to fetch source for https://go.googlesource.com/sync: unable to get repository: Cloning into '/tmp/tmprh1ofsfv/pkg/dep/sources/https---go.googlesource.com-sync'...
fatal: unable to access 'https://go.googlesource.com/sync/': The requested URL returned error: 502
: command failed: [git clone --recursive -v --progress https://go.googlesource.com/sync /tmp/tmprh1ofsfv/pkg/dep/sources/https---go.googlesource.com-sync]: exit status 128

@filizt
Copy link
Contributor Author

filizt commented Jun 14, 2019

@shahabhijeet I looked at the failing jobs. We had the same failures in our previous PR and they were suppressed and the PR was merged. You can find it here: #5872.

These failures are specifically around the ParameterValueBase property and its polymorphic relation to ParameterValue. It seems like the model validation tool is not seeing this relationship and complaining that the "value" parameter in the request examples are not complaint. What would be a recommended way to solve this issue?

Please let me know if we can also suppress these failures for this PR. I can create a separate PR to fix the above mentioned issue that causes some of the checks to fail.

@shahabhijeet
Copy link
Member

@sergey-shandar can you comment on the above comment about model validation "potentially" showing false negatives.

@sergey-shandar
Copy link
Contributor

sergey-shandar commented Jun 18, 2019

@filizt In OpenAPI 2.0 if additionalProperties is not specified, then additional properties are not allowed. If the ParameterValueBase is a polymorphic type, please use "additionalProperties": { "type": ... }.

@filizt
Copy link
Contributor Author

filizt commented Jun 19, 2019

@filizt In OpenAPI 2.0 if additionalProperties is not specified, then additional properties are not allowed. If the ParameterValueBase is a polymorphic type, please use "additionalProperties": { "type": ... }.

@sergey-shandar I am not sure if I understand how "additionalProperties" will fix the issue here. Adding @majastrz to help explain the issue a bit more.

@majastrz
Copy link
Member

@filizt In OpenAPI 2.0 if additionalProperties is not specified, then additional properties are not allowed. If the ParameterValueBase is a polymorphic type, please use "additionalProperties": { "type": ... }.

@sergey-shandar I am not sure if I understand how "additionalProperties" will fix the issue here. Adding @majastrz to help explain the issue a bit more.

@sergey-shandar, we're basically looking for the best way to model the following JSON:

"parameter": {
  "foo": {
    "value": 42
  },
  "bar": {
    "reference": {
       ...
    }
  }
}

The parameter value should either be have a reference or value property set, but (ideally) not both. One workaround I can think of is just to create a ParameterValue definition in the swagger with both value and reference properties as optional, but I'm hoping you can recommend some better solution.

@filizt
Copy link
Contributor Author

filizt commented Jun 19, 2019

@shahabhijeet Can we suppress the checks and merge the changes? We would not want to hold the PR since the issue discussed here is not new and is not caused by the change in this PR. I can create a separate item in the repo's "Issues" section to discuss this issue in detail. Let me know if you want me to go ahead and create a new issue.

@shahabhijeet
Copy link
Member

@sergey-shandar will be the right person who can answer the question about supression.

@sergey-shandar
Copy link
Contributor

@majastrz for OpenAPI 2.0 it is the way. Create a type that has two optional properties. Open-API 3.0 should support "oneOf" but we don't support 3.0 yet.

@sergey-shandar
Copy link
Contributor

sergey-shandar commented Jun 20, 2019

@filizt @majastrz could you create a new PR to fix model validation issues and we will merge this?

@filizt
Copy link
Contributor Author

filizt commented Jun 20, 2019

@filizt @majastrz could you create a new PR to fix model validation issues and we will merge this?

Created the following PR to follow up on a fix for the model validation errors: #6394

Can you merge this one please?

@sergey-shandar
Copy link
Contributor

sergey-shandar commented Jun 20, 2019

Fix for OAV tracked in Azure/oav#431

@jhendrixMSFT jhendrixMSFT merged commit 2f48d30 into Azure:master Jun 20, 2019
leni-msft pushed a commit to leni-msft/azure-rest-api-specs that referenced this pull request May 13, 2022
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.

8 participants