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

Add specs for machine learning services provider #3135

Merged
merged 6 commits into from
May 30, 2018

Conversation

ragargms
Copy link
Member

@ragargms ragargms commented May 24, 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 May 24, 2018

Automation for azure-libraries-for-java

Nothing to generate for azure-libraries-for-java

@ragargms
Copy link
Member Author

@sergey-shandar This is the PR which I now moved to public repo

@AutorestCI
Copy link

AutorestCI commented May 24, 2018

Automation for azure-sdk-for-python

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

@sergey-shandar sergey-shandar self-assigned this May 24, 2018
@annatisch
Copy link
Member

@ragargms - Has this PR already been reviewed in the private repo? If so - could you please link to that PR here?

Additionally - I will go ahead and add the Python SDK generation to the readme.

@ragargms
Copy link
Member Author

ragargms commented May 24, 2018

@sergey-shandar already started to look at the private repo PR. Since this is not a secret service, we decided to create PR against the public repo. Also, do you have references as to how I can add Python SDK generation to the readme? @annatisch

Here is the private repo PR link https://github.com/Azure/azure-rest-api-specs-pr/pull/480

@annatisch annatisch added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label May 24, 2018
@ragargms
Copy link
Member Author

@ravbhatnagar for ARM review. I think we reviewed most of this already offline when we were getting started with the service.

@AutorestCI
Copy link

AutorestCI commented May 25, 2018

Automation for azure-sdk-for-node

Nothing to generate for azure-sdk-for-node

@AutorestCI
Copy link

AutorestCI commented May 25, 2018

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

"$ref": "#/parameters/ComputeDefinitionParameter"
}
],
"responses": {
Copy link
Contributor

Choose a reason for hiding this comment

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

200 OK response to differentiate between created or updated

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

}
],
"responses": {
"202": {
Copy link
Contributor

Choose a reason for hiding this comment

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

200 response

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

}
}
},
"post": {
Copy link
Contributor

Choose a reason for hiding this comment

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

POST on this URL will not work. the action segment is missing.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

"readOnly": true,
"type": "string"
},
"userStorageArmId": {
Copy link
Contributor

Choose a reason for hiding this comment

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

lets not use short forms like ARM for property names. ResourceId works?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@AutorestCI
Copy link

AutorestCI commented May 29, 2018

Automation for azure-sdk-for-go

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

@ravbhatnagar ravbhatnagar added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels May 29, 2018
@ravbhatnagar
Copy link
Contributor

Looks good. Signing off!

"201": {
"description": "Compute creation or update initiated.",
"headers": {
"Azure-AsyncOperation": {
Copy link
Member

Choose a reason for hiding this comment

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

If this operation is a polling/async operation - then it should use the long-running-operation extension.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added

"202": {
"description": "Compute deletion initiated.",
"headers": {
"Azure-AsyncOperation": {
Copy link
Member

Choose a reason for hiding this comment

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

If this operation is a polling/async operation - then it should use the long-running-operation extension.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added

"OperationalizationClusters",
"MachineLearningComputes"
],
"operationId": "MachineLearningCompute_ListKeys",
Copy link
Member

Choose a reason for hiding this comment

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

The operation ID could be renamed to MachineLearningCompute_GetKeys - as it's not really a list operation.

Copy link
Member Author

Choose a reason for hiding this comment

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

We already have clients using this contract. Is it ok to keep it this way? We got this reviewed with ARM. Changing this will require us significant work to update all our clients and first party services.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah. Sorry. You are just asking to change the operation id not the route, that can be done. Fixed

"default": {
"description": "Error response describing why the operation failed",
"schema": {
"$ref": "#/definitions/ErrorResponseWrapper"
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to rename this ErrorResponseWrapper to something like MachineLearningServiceError?
It's not blocking - but the naming is a little non-intuitive for a client development experience. Though it's up to you :)

@annatisch
Copy link
Member

This looks okay to me. @sergey-shandar - do you have any further comments?

Copy link
Contributor

@sergey-shandar sergey-shandar left a comment

Choose a reason for hiding this comment

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

LGTM

@sergey-shandar sergey-shandar merged commit f0d8a6b into Azure:master May 30, 2018
bashahee pushed a commit that referenced this pull request Jun 3, 2018
* Add specs for machine learning services provider

* Added python config to readme

* Added repo generation for Python

* Addressed feedback

* Address more comments
hovsepm pushed a commit that referenced this pull request Jun 8, 2018
* One file for server and db apis

Make one file which is blobAuditing.json that contains both server & db apis + update readme.md and add operations.json

* Add extended server auditing settings API

* Fixed operation id.

* Enable Database Auditing Filtiring With a Predicate Expression

New API which enables database auditing filtering by a where clause.

* Enable Database Auditing Filtiring With a Predicate Expression

New API which enables database auditing filtering by a where clause.
In addition, existing related APIs swaggers were unified under same Public API name.

* Enable Database Auditing Filtiring With a Predicate Expression

New API which enables database auditing filtering by a where clause.
In addition, existing related APIs swaggers were unified under same Public API name.

* Fixing examples of existing APIs and modifying blobAuditing Swagger file

storageAccountAccessKey was removed from requests response, and examples were modified accordingly.

* Remove description of predicateExpression related exceptions from old APIs description.

Fixing a small issue for client readability.
AuditingSettings and ExtendedAuditingSettings APIs have a mutual exceptions description. Description which mentions predicateExpression parameter in all APIs.
After this fix, predicateExpression related exceptions will not be mentioned on old APIs.

* Moving files to preview/stable as appropriate.

* Modifying swagger and examples of 2017 BlobAuditing APIs (#421)

* 2017 Blob Auditing APIs - Swagger and examples.

Modifying swagger and examples of 2017 blob auditing APIs.

* Resetting examples of DatabaseBlobAuditing API to 2017

Resetting examples of DatabaseBlobAuditing API to 2017

* Fixing follwing comments

Fixing follwing comments

* Fixing build errors

Fixing build errors

* Fix build errors

Fix build errors

* Fixing comments

fixing comments

* Removed 2017 database API

Removed 2017 database API

* Revert "Fixing comments"

This reverts commit c09f9f6.

* Modifying description of APIs

Modifying description of APIs after generating new swagger.

* Modifying readme.md.

Modifying readme.md.

* Updating blobAuditing Json files.

Updating blobAuditing Json files.

* Temp change

Temp change

* Final commit

Final commit

* Fix Duplicated model name with non-identical definitions build error

Fix Duplicated model name with non-identical definitions

* Fix Duplicated model name with non-identical definitions build error

This reverts commit 616851d and commits fixes.

* Fix Automation Duplicated model name with non-identical definitions

Fix Automation Duplicated model name with non-identical definitions

* One file for server and db apis

Make one file which is blobAuditing.json that contains both server & db apis + update readme.md and add operations.json

* Add extended server auditing settings API

* Fixed operation id.

* Enable Database Auditing Filtiring With a Predicate Expression

New API which enables database auditing filtering by a where clause.

* Enable Database Auditing Filtiring With a Predicate Expression

New API which enables database auditing filtering by a where clause.
In addition, existing related APIs swaggers were unified under same Public API name.

* Enable Database Auditing Filtiring With a Predicate Expression

New API which enables database auditing filtering by a where clause.
In addition, existing related APIs swaggers were unified under same Public API name.

* Fixing examples of existing APIs and modifying blobAuditing Swagger file

storageAccountAccessKey was removed from requests response, and examples were modified accordingly.

* Remove description of predicateExpression related exceptions from old APIs description.

Fixing a small issue for client readability.
AuditingSettings and ExtendedAuditingSettings APIs have a mutual exceptions description. Description which mentions predicateExpression parameter in all APIs.
After this fix, predicateExpression related exceptions will not be mentioned on old APIs.

* Moving files to preview/stable as appropriate.

* Modifying swagger and examples of 2017 BlobAuditing APIs (#421)

* 2017 Blob Auditing APIs - Swagger and examples.

Modifying swagger and examples of 2017 blob auditing APIs.

* Resetting examples of DatabaseBlobAuditing API to 2017

Resetting examples of DatabaseBlobAuditing API to 2017

* Fixing follwing comments

Fixing follwing comments

* Fixing build errors

Fixing build errors

* Fixing comments

fixing comments

* Removed 2017 database API

Removed 2017 database API

* Revert "Fixing comments"

This reverts commit c09f9f6.

* Modifying description of APIs

Modifying description of APIs after generating new swagger.

* Modifying readme.md.

Modifying readme.md.

* Updating blobAuditing Json files.

Updating blobAuditing Json files.

* Temp change

Temp change

* Final commit

Final commit

* Fix Python conf of servermanager (#3140)

* Fix ServerManager Py Conf (#3141)

* Plug SwaggerToSdk to LogAnalyticsMgmt (#3142)

* [Storage] Support Management policy on new api version 2018-03-01-preview (#3137)

* [Storage] Add new API version 2018-03-01-preview

* [Storage] Update rest version 2018-03-01-preview

* [Storage] Support Management Policy

* [Storage] Update since code review comments

* Add Python conf

* [Storage] Add managment policy doc link to policy description.

* [Storage] modify as review comments

* RSA key size examples, ECC descriptions (#3136)

* RSA key size examples, ECC descriptions

* remove preview changes for now

* Added GroupType property in HybridWorkerGroup (#3132)

* Watcher resource swagger specs. For more details on the Usage of the resource: https://docs.microsoft.com/en-us/azure/automation/automation-watchers-tutorial (#3102)

* Watcher specs

* using the same model for get/put input and output.

* Fix storage Python conf (#3148)

* Fix for Azure Networking swagger 2018-04-01 version (#3146)

* Adding swagger for new api-version consumption 2018-05-31, introducing new Forecast api swagger (#3114)

* Forecast Api swagger

* consumption readme file update for the new swagger version

* addressing review comment on adding nextLink

* fixing build error

* Addressed feedback on enum and fixed a typo

* Stop using unobscured credentials

* Fix Duplicated model name with non-identical definitions build error

Fix Duplicated model name with non-identical definitions

* Fix go tag for storage management plane 2018-03-02 api version (#3156)

* Refactoring Job Definitions (#3116)

* MySQL/PostgreSQL VNET with API Version 2017-12-01 for Public Preview. (#3138)

* Fix Python conf

* Adding the VNET Rest APIs to 2017-12-01 version.

Adding the VNET Rest APIs to 2017-12-01 version.

* Add specs for machine learning services provider (#3135)

* Add specs for machine learning services provider

* Added python config to readme

* Added repo generation for Python

* Addressed feedback

* Address more comments

* Adding Security RP API swagger (#3144)

* Reverted methods in vpn gateways. (#3163)

* Fix Duplicated model name with non-identical definitions build error

This reverts commit 616851d and commits fixes.

* Fix Automation Duplicated model name with non-identical definitions

Fix Automation Duplicated model name with non-identical definitions

* Revert "Fix Duplicated model name with non-identical definitions build error"

This reverts commit d1c40f7.

* Revert "Fix Duplicated model name with non-identical definitions build error"

This reverts commit 2c1261e.

* Update readme.md file

Update readme.md file
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 review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants