Skip to content

Azure Machine Learning WebServicesRP new API version#2921

Closed
NonStatic2014 wants to merge 16 commits into
Azure:AutoRestfrom
NonStatic2014:new_version
Closed

Azure Machine Learning WebServicesRP new API version#2921
NonStatic2014 wants to merge 16 commits into
Azure:AutoRestfrom
NonStatic2014:new_version

Conversation

@NonStatic2014
Copy link
Copy Markdown
Contributor

@NonStatic2014 NonStatic2014 commented Mar 10, 2017

Description

Azure Machine Learning Web Services RP is publishing a new version of API. The .Net SDK needs to be regenerated from a new swagger spec. The pull request of swagger spec change is here: Azure/azure-rest-api-specs#1019. Compare with the existing preview version, this is a breaking change according to breaking changes

Scenario Tests have been updated and two new test cases were added to cover new scenarios.


This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

  • Title of the pull request 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 more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

SDK Generation Guidelines

  • If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
  • The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as a link to the swagger spec, used to generate the code.
  • The project.json and AssemblyInfo.cs files have been updated with the new version of the SDK.

@markcowl
Copy link
Copy Markdown
Member

@NonStatic2014 Please change the PR title to be more descriptive. You realize that you checked a box saying the PR had a descriptive title, right? :-)

@NonStatic2014 NonStatic2014 changed the title New version Azure Machine Learning WebServicesRP new API version Mar 10, 2017
@NonStatic2014
Copy link
Copy Markdown
Contributor Author

Good catch Mark! I reopen the web page but forget to update the title. Now it is updated.

@NonStatic2014
Copy link
Copy Markdown
Contributor Author

I have run the following command locally and exit successfully. Not sure why lab build keep failing.

.\CredentialScanner.exe -I D:\code\GitHub\azure-sdk-for-net\src\ResourceManagement\MachineLearning -S Searchers\buildsearchers.xml -O d:\temp\scan1 -t -v > Result.txt
Result.txt

Copy link
Copy Markdown
Member

@cormacpayne cormacpayne left a comment

Choose a reason for hiding this comment

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

@NonStatic2014 Hey Klein, some minor comments that need to be resolved before the swagger gets merged and we can review the generated code.

@@ -1,5 +1,5 @@
{
"version": "0.10.0-preview",
"version": "1.0.0",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@NonStatic2014 this should be version 1.0.0-preview (or 0.11.0-preview) since you are introducing breaking changes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This version is going to be the GA version, so we expect it is called 1.0.0.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We still require the package to be in preview when breaking changes are introduced to the SDK, and after enough time to ensure it is stable, you can remove the -preview suffix and re-publish a new version

set autoRestVersion=0.17.0-Nightly20160824

set webServicesSpecFile="https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-machinelearning/2016-05-01-preview/swagger/webservices.json"
set webServicesSpecFile="https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-machinelearning/2017-01-01/swagger/webservices.json"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@NonStatic2014 please use the URL containing the commit Id when the spec gets merged instead of the URL containing the branch

@@ -8,7 +8,7 @@ setlocal

set autoRestVersion=0.17.0-Nightly20160824
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@NonStatic2014 AutoRest is now at version 1.0.x. Please refer to aka.ms/autorest/install for more information on how to install and use the latest version(s).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Then please ignore this cmd file and we are going to follow the new instruction generate code locally.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We would still prefer if you updated this file with the version of AutoRest used (even if it's from the new instructions); it let's us know that you aren't using an outdated version of AutoRest if we were to just look at this file

@shahabhijeet
Copy link
Copy Markdown
Contributor

@NonStatic2014 please reopen this PR once the swagger PR is merged.
We are trying to accurately asses our SLA and opening a PR which is not ready to be reviewed. We will not review SDK PR unless swagger PR is merged.

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.

5 participants