Skip to content

Add cognitive service AnomalyDetector SDK with unittests#5276

Merged
dsgouda merged 3 commits intoAzure:masterfrom
moreOver0:zhuxia/anomaly_finder
Mar 15, 2019
Merged

Add cognitive service AnomalyDetector SDK with unittests#5276
dsgouda merged 3 commits intoAzure:masterfrom
moreOver0:zhuxia/anomaly_finder

Conversation

@moreOver0
Copy link
Contributor

@moreOver0 moreOver0 commented Feb 22, 2019

Azure/azure-rest-api-specs#5295 This is the related REST spec API PR.

@moreOver0
Copy link
Contributor Author

moreOver0 commented Feb 25, 2019

@yangyuan Please take a reivew except the file cognitiveservices_data-plane_AnomalyDetector.txt because it's not real log when generating code. I will update this file when Azure/azure-rest-api-specs#5295 get merged.

@moreOver0 moreOver0 changed the title Add cognitive service AnomalyFinder SDK with unittests Add cognitive service AnomalyDetector SDK with unittests Feb 28, 2019
@dsgouda
Copy link
Contributor

dsgouda commented Feb 28, 2019

@moreOver0 please squash the commits for a cleaner commit history

fullfill .net solution structure

add unit tests

update PackageReleaseNotes

copy resource "PreserveNewest"

change  service name

fix property name in response

change sdk version to 1.0.0.0

update change from swagger, make non nullable

update PackageReleaseNotes

update swagger change

re-generate code from zhuxia/ad from repo moreOver0/azure-rest-api-specs

updates
Copy link
Contributor

@dsgouda dsgouda left a comment

Choose a reason for hiding this comment

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

Approved in principle. Will approve and merge once the code is generated from merged REST spec

@shahabhijeet
Copy link
Contributor

@AlexGhiondea can you approve this as this is dataplane PR.

Copy link
Contributor

@AlexGhiondea AlexGhiondea left a comment

Choose a reason for hiding this comment

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

I have a question about the package version.

Commencing code generation
Generating CSharp code
Executing AutoRest command
cmd.exe /c autorest.cmd https://github.com/moreOver0/azure-rest-api-specs/blob/zhuxia/ad/specification/cognitiveservices/data-plane/AnomalyDetector/readme.md --csharp --version=latest --reflect-api-versions --csharp-sdks-folder=D:\src\azure-sdk-for-net\src\SDKs
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this supposed to be added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean this line cmd.exe /c autorest.cmd https://github.com/moreOver0/azure-rest-api-specs/blob/zhuxia/ad/specification/cognitiveservices/data-plane/AnomalyDetector/readme.md --csharp --version=latest --reflect-api-versions --csharp-sdks-folder=D:\src\azure-sdk-for-net\src\SDKs ?

I generated the C# code from my own branch because my swagger PR is not merged now. I will update this after the swagger PR get merged.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry -- I was asking about the entire text file.

Copy link
Contributor Author

@moreOver0 moreOver0 Mar 15, 2019

Choose a reason for hiding this comment

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

I thought I should add this file. It is an auto generated log file when I use generate.ps1 to generate code. I believe this file is target to record repo, branch and commit id.

@dsgouda What do you think about this file? Should I add this file?

Copy link
Contributor

Choose a reason for hiding this comment

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

@AlexGhiondea This is a log file we generate when generating the SDK using scripts. Helps us review what specs were used for it.

<PackageTags>Azure AnomalyDetector;</PackageTags>
<PropertyGroup>
<PackageId>Microsoft.Azure.CognitiveServices.AnomalyDetector</PackageId>
<Version>0.8.0-preview</Version>
Copy link
Contributor

Choose a reason for hiding this comment

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

I see you are bumping down the version number and changing the name. Any reason to pick 0.8.0 here instead of something else (like 0.1.0)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shahabhijeet merged some project structures in #5464 because he didn't aware that there already has a ready SDK PR. So I solved the conflicts after that. Please ignore all these changes.

@dsgouda suggested me to use 0.8.0-preivew in previous comments.

Copy link
Contributor

Choose a reason for hiding this comment

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

@AlexGhiondea For brand new RPs we recommend starting from 0.8.0-preview so that when they are ready to release a stable version they can start from 1.0.0. we apply the same guidelines when working with the next preview versions (1.8.0-preview-2.0.0)

@dsgouda
Copy link
Contributor

dsgouda commented Mar 15, 2019

@AlexGhiondea I have answered the questions, let us know if this good to merge

@moreOver0
Copy link
Contributor Author

@dsgouda I have updated the generate.ps1 and session records since the swagger PR got merged. You can merge this PR when you and Alex think it's OK.

@AlexGhiondea
Copy link
Contributor

LGTM!

@dsgouda dsgouda merged commit efe8af8 into Azure:master Mar 15, 2019
mentat9 pushed a commit to mentat9/azure-sdk-for-net that referenced this pull request Jun 10, 2019
* add anomaly detector sdk with unit tests

fullfill .net solution structure

add unit tests

update PackageReleaseNotes

copy resource "PreserveNewest"

change  service name

fix property name in response

change sdk version to 1.0.0.0

update change from swagger, make non nullable

update PackageReleaseNotes

update swagger change

re-generate code from zhuxia/ad from repo moreOver0/azure-rest-api-specs

updates

* update generate.ps1 and session records
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