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

Updating Data Classification Swaggers. #5148

Merged
merged 9 commits into from
Feb 6, 2019
Merged

Updating Data Classification Swaggers. #5148

merged 9 commits into from
Feb 6, 2019

Conversation

bashahee
Copy link
Contributor

@bashahee bashahee commented Feb 4, 2019

This PR updates Data Classification swagger files and examples. This is not a breaking change, as reference to these files was not present in the readme.me file, hence clients were not generated yet.

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.

@AutorestCI
Copy link

AutorestCI commented Feb 4, 2019

Automation for azure-sdk-for-js

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

@AutorestCI
Copy link

AutorestCI commented Feb 4, 2019

Automation for azure-sdk-for-ruby

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

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Feb 4, 2019

Automation for azure-sdk-for-python

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

@AutorestCI
Copy link

AutorestCI commented Feb 4, 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#4042

@AutorestCI
Copy link

AutorestCI commented Feb 4, 2019

Automation for azure-sdk-for-node

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

@AutorestCI
Copy link

AutorestCI commented Feb 4, 2019

Automation for azure-sdk-for-java

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

Copy link
Contributor

@veronicagg veronicagg left a comment

Choose a reason for hiding this comment

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

CI is failing syntax and semantic checks, please take a look at those first, so the other tools can properly execute. If you need help, please let me know. Thanks!

@bashahee
Copy link
Contributor Author

bashahee commented Feb 5, 2019

@veronicagg Automation for azure-sdk-for-java is failing since I added - ./Microsoft.Sql/preview/2017-03-01-preview/sensitivityLabels.json under package-pure-2017-03-preview in the readme.md file.

@veronicagg
Copy link
Contributor

@bashahee there's probably a class name that's duplicated in that version when you add the file, so it's colliding on SDK generation - "Duplicate File Generation".
@anuchandy could you help with Java SDK generation for ./Microsoft.Sql/preview/2017-03-01-preview/sensitivityLabels.json under package-pure-2017-03-preview ?

@veronicagg
Copy link
Contributor

@jaredmoo do you want to look at/approve this PR?
@bashahee have the changes and new files been reviewed by ARM team?
Apart from Java generation, rest looks good to me.

Copy link
Contributor

@veronicagg veronicagg left a comment

Choose a reason for hiding this comment

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

See comments inline.

@bashahee
Copy link
Contributor Author

bashahee commented Feb 5, 2019

@veronicagg I am not introducing new APIs, these APIs are already in production. I am updating swagger files in order to generate SDK clients, as part of my work on implementing PowerShell Cmdlets.
No source code was changed, apart from modifying examples.

@bashahee
Copy link
Contributor Author

bashahee commented Feb 6, 2019

@veronicagg Can this PR be merged now?

@anuchandy
Copy link
Member

@bashahee Identified an issue in java codegen and created an issue here Azure/autorest.java#323.

However fixing it requires some time, meantime to unblock your PR, I've updated readme Java section and opened this PR https://github.com/bashahee/azure-rest-api-specs/pull/1, could you merge it?

@veronicagg
Copy link
Contributor

@bashahee I was waiting on the java code gen issue, please take a look at the PR suggested by @anuchandy , if it makes sense to you to do the rename to avoid collisions, please merge that in, and I'll approve/merge this PR. Thanks!

@veronicagg veronicagg merged commit d7a6444 into Azure:master Feb 6, 2019
@jhendrixMSFT
Copy link
Member

@AutorestCI regenerate azure-sdk-for-go

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.

7 participants