Skip to content

Swagger closure for Azure Migration Hub #4481

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.

Sign up for GitHub

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

Jump to bottom
Merged
praries880 merged 35 commits into from
Jul 1, 2019
Merged

Swagger closure for Azure Migration Hub #4481

praries880 merged 35 commits into from
Jul 1, 2019

Conversation

@peetlotla
Copy link
Contributor

@peetlotla peetlotla commented Nov 15, 2018

Incorporated changes as per ARM review.

@peet-lotla
Swagger coverage for Azure Migrate Hub operations
6b1ced7
@peet-lotla
Adding Operation_List.json to examples
1244a35
@peet-lotla
adding operation result definitions to migrate.json
b7bb635
@peet-lotla
Update specification/migrateprojects/resource-manager/Microsoft.Migra…
eb38afe 
…te/preview/2018-09-01-preview/migrate.json

Fixing the JSON schema validation for migrate.json
@peet-lotla
Update specification/migrateprojects/resource-manager/readme.md
ae58786 
Fixing the code generation path for preview swagger.
@peet-lotla
Update specification/migrateprojects/resource-manager/Microsoft.Migra…
883fa55 
…te/preview/2018-09-01-preview/migrate.json

Fixing potential SDK and security errors.
@peet-lotla
Update specification/migrateprojects/resource-manager/Microsoft.Migra…
002f6df 
…te/preview/2018-09-01-preview/migrate.json

Fixing potential new SDK errors.
@peet-lotla
Proving x-ms-exmaples for response/request payloads for operations.
7c87c69
@peet-lotla
Fixing schema validation for azure-sdk-for-go.
0c80987
@peet-lotla
Fixing schema validation for azure-sdk-for-go.
3b7b463
@peet-lotla
Added missing patch and delete operations per the feedback.
1faf797
@peet-lotla
Swagger fixes for azure-sdk-for-go
07a3678
@peet-lotla
Swagger fixes for azure-sdk-for-go
f4b032d
@peet-lotla
Swagger fixes for azure-sdk-for-go
251876f
@peet-lotla
Fixing schema validation for azure-sdk-for-go.
70edfe5
@peet-lotla
Adding delete method for migrate errors.
461c921
@peet-lotla
Updates as per ARM review.
7ca6abc
@peet-lotla
Updates as per ARM feedback.
772d61f
@peet-lotla
Updates as per ARM feedback.
c19dd1f
@openapi-portal-comment
Copy link

openapi-portal-comment bot commented Nov 15, 2018

If you're a MSFT employee, click this link
to view this PR's validation status on our new OpenAPI Hub spec management tool.

@AutorestCI
Copy link

AutorestCI commented Nov 15, 2018 •
edited
Loading

Automation for azure-sdk-for-node

Nothing to generate for azure-sdk-for-node

@AutorestCI
Copy link

AutorestCI commented Nov 15, 2018 •
edited
Loading

Automation for azure-sdk-for-js

Nothing to generate for azure-sdk-for-js

@AutorestCI
Copy link

AutorestCI commented Nov 15, 2018 •
edited
Loading

Automation for azure-sdk-for-python

Nothing to generate for azure-sdk-for-python

@AutorestCI
Copy link

AutorestCI commented Nov 15, 2018 •
edited
Loading

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

@AutorestCI
Copy link

AutorestCI commented Nov 15, 2018 •
edited
Loading

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@AutorestCI AutorestCI mentioned this pull request Nov 15, 2018
Closed
@AutorestCI
Copy link

AutorestCI commented Nov 15, 2018 •
edited
Loading

Automation for azure-sdk-for-go

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

@peetlotla peetlotla changed the title Peet Swagger closure for Azure Migration Hub Nov 15, 2018
@AutorestCI AutorestCI mentioned this pull request May 29, 2019
Merged
@peetlotla
Copy link
Contributor Author

peetlotla commented Jun 13, 2019

@peetlotla - Are there changes that need a ARM re-review? I am checking since we reviewed and signed off on this few months ago.

@ravbhatnagar - The swagger spec has changes related to Databases integration with Azure Migrate Hub. Also, as discussed in the mail I have used the discriminator pattern to include sub classes in the swagger spec.

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jun 18, 2019 •
edited
Loading

SDK Automation [Logs] (Generated from 194da23)

Pending Go: Azure/azure-sdk-for-go
  • Package generation pending.

@peetlotla
Copy link
Contributor Author

peetlotla commented Jun 18, 2019

ARM feedback based on discussion with @ravbhatnagar:

  1. Database instance id can be used as the arm name for the instance
  2. Solution name in the properties needs to be fully qualified ARM name
  3. Database instances and databases can have the parent child hierarchy.

@peetlotla
Copy link
Contributor Author

peetlotla commented Jun 18, 2019 •
edited
Loading

ARM feedback based on discussion with @ravbhatnagar:

  1. Database instance id can be used as the arm name for the instance
  2. Solution name in the properties needs to be fully qualified ARM name
  3. Database instances and databases can have the parent child hierarchy.

1 and 2 are good to do but not a breaking change.
For 3 - Databases view is important for CX. Customers/Portal would want to skip the instance scanning while listing the databases. So we want to keep the current implementation as is.

Since GA is approaching we would like to go ahead and merge this PR. As discussed the feedback is not a breaking change so we need to work on this feedback post GA or whenever the next update happens for this resource.

@nschonni
Copy link
Contributor

nschonni commented Jun 18, 2019

You can get the spellcheck build to pass by adding the new words to https://github.com/Azure/azure-rest-api-specs/blob/master/custom-words.txt

@ravbhatnagar
Copy link
Contributor

ravbhatnagar commented Jun 19, 2019

As part of review done on 6/18, we just reviewed the delta as presented by service team. This included reviewing the two new types being added - databases and databaseInstances.
Service team has agreed to make the changes (#1 and #2) after the GA milestone since these are non-breaking changes. #3 is still not committed but will be explored. Signing off on this change from ARM side.

@peetlotla peetlotla changed the title [Do not merge] Swagger closure for Azure Migration Hub Swagger closure for Azure Migration Hub Jun 19, 2019
@peetlotla
Copy link
Contributor Author

peetlotla commented Jun 19, 2019 via email

@peetlotla
Copy link
Contributor Author

peetlotla commented Jun 20, 2019 via email

@praries880 praries880 removed the DoNotMerge <valid label in PR review process> use to hold merge after approval label Jul 1, 2019
@praries880 praries880 merged commit 7d74c8d into Azure:master Jul 1, 2019
@peetlotla peetlotla deleted the peet branch July 4, 2019 07:59
celikcigdem pushed a commit to celikcigdem/azure-rest-api-specs that referenced this pull request Jul 17, 2019
@peetlotla @celikcigdem
Swagger closure for Azure Migration Hub (Azure#4481)
122e7ff 
* Swagger coverage for Azure Migrate Hub operations

* Adding Operation_List.json to examples

* adding operation result definitions to migrate.json

* Update specification/migrateprojects/resource-manager/Microsoft.Migrate/preview/2018-09-01-preview/migrate.json

Fixing the JSON schema validation for migrate.json

* Update specification/migrateprojects/resource-manager/readme.md

Fixing the code generation path for preview swagger.

* Update specification/migrateprojects/resource-manager/Microsoft.Migrate/preview/2018-09-01-preview/migrate.json

Fixing potential SDK and security errors.

* Update specification/migrateprojects/resource-manager/Microsoft.Migrate/preview/2018-09-01-preview/migrate.json

Fixing potential new SDK errors.

* Proving x-ms-exmaples for response/request payloads for operations.

* Fixing schema validation for azure-sdk-for-go.

* Fixing schema validation for azure-sdk-for-go.

* Added missing patch and delete operations per the feedback.

* Swagger fixes for azure-sdk-for-go

* Swagger fixes for azure-sdk-for-go

* Swagger fixes for azure-sdk-for-go

* Fixing schema validation for azure-sdk-for-go.

* Adding delete method for migrate errors.

* Updates as per ARM review.

* Updates as per ARM feedback.

* Updates as per ARM feedback.

* Updates as per ARM feedback.

* Making changes to existing PR for AMH

* Fixnig the semantic errors

* Fixnig the semantic errors

* Fixing semantic errors

* Fixing semantic errors

* Fixing semantic errors

* Fixing semantic errors

* Fixing semantic errors

* Fixing SDK errors

* Fixing SDK errors

* Changing the resource names to camel case as per ARM review

* Changing the resource names to camel case as per ARM review

* Changing the resource names to camel case as per ARM review

* Fix model validation errors

* Modifying the team email address
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@praries880 praries880 praries880 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@praries880 praries880

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@peetlotla @AutorestCI @azuresdkci @ravbhatnagar @salameer @praries880 @nschonni @peet-lotla