Skip to content

Aro 2019-09-30 preview API Version#7211

Merged
raych1 merged 12 commits intoAzure:masterfrom
JackQuincy:aro-2019-09-30-preview
Oct 7, 2019
Merged

Aro 2019-09-30 preview API Version#7211
raych1 merged 12 commits intoAzure:masterfrom
JackQuincy:aro-2019-09-30-preview

Conversation

@JackQuincy
Copy link
Contributor

@JackQuincy JackQuincy commented Sep 13, 2019

Adding swagger spec for the newest API Version for ARO.

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 Sep 13, 2019

In Testing, Please Ignore

[Logs] (Generated from aa9913e, Iteration 13)

Succeeded Python: test-repo-billy/azure-sdk-for-python [Logs] [Diff]
Failed Java: test-repo-billy/azure-sdk-for-java [Logs] [Diff]
Succeeded Go: test-repo-billy/azure-sdk-for-go [Logs] [Diff]
Warning JavaScript: test-repo-billy/azure-sdk-for-js [Logs] [Diff]
  • Warning @azure/arm-containerservice [Logs]
Succeeded .NET: test-repo-billy/azure-sdk-for-net [Logs] [Diff]
Succeeded Ruby: test-repo-billy/azure-sdk-for-ruby [Logs] [Diff]

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Sep 13, 2019

Automation for azure-sdk-for-python

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

@AutorestCI
Copy link

AutorestCI commented Sep 13, 2019

Automation for azure-sdk-for-go

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

@raych1 raych1 added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Sep 16, 2019
@raych1
Copy link
Member

raych1 commented Sep 16, 2019

@JackQuincy, please copy the previous version API swagger as first commit then work on top of it for the new change when adding new API version. It will be easier for the review on the new change. Another thing, it's better to have the folder name called as ...nerService/preview/2019-09-30/... without literal preview after API version so that it will be easier to move the exact whole folder to stable version.

Copy link
Contributor

@KrisBash KrisBash left a comment

Choose a reason for hiding this comment

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

I compared with 2018-09-30 preview, and the actual API changes seem fine. I think the only required fix is the version property in the swagger spec

"info": {
"title": "ContainerServiceClient",
"description": "The Container Service Client.",
"version": "2019-04-30"
Copy link
Contributor

Choose a reason for hiding this comment

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

Version does not match folder name or PR title.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I'll fix this soon

@JackQuincy
Copy link
Contributor Author

Internally we have decided to add an enabled disabled flag here as well so that we can allow the user to remove the logging integration. And that change is waiting on our partner team in redhat. Once that is finalized I'll update this pr. Should be by EOW

@JackQuincy
Copy link
Contributor Author

took a little longer then expected but this is now updated with the final schema for this version and ready for review

@JackQuincy
Copy link
Contributor Author

@raych1 this is ready for review again sorry for the long time between revisions

@raych1 raych1 requested a review from KrisBash September 27, 2019 01:21
},
"monitorProfile": {
"workspaceResourceID": "/subscriptions/subid1/resourcegroups/rg1/providers/Microsoft.OperationalInsights/workspaces/workspacename1"
"enabled": true,
Copy link
Member

Choose a reason for hiding this comment

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

Please correct indent.


### Tag: package-2019-09-preview-only and python

These settings apply only when `--tag=package-2019-08-preview-only --python` is specified on the command line.
Copy link
Member

Choose a reason for hiding this comment

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

Typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

copy paste error. fixing

@raych1
Copy link
Member

raych1 commented Sep 27, 2019

@KrisBash , please review the changes, thanks.

Copy link
Contributor

@KrisBash KrisBash left a comment

Choose a reason for hiding this comment

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

LGTM

@KrisBash KrisBash added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed ARMChangesRequested WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Oct 2, 2019
@JackQuincy
Copy link
Contributor Author

@raych1 this should be ready for review again. Sorry for slow turn around.

@raych1 raych1 merged commit a390b90 into Azure:master Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants