Skip to content

Custom vision 2.0#4292

Merged
dsgouda merged 11 commits intoAzure:psSdkJson6from
areddish:custom-vision-2.0
May 7, 2018
Merged

Custom vision 2.0#4292
dsgouda merged 11 commits intoAzure:psSdkJson6from
areddish:custom-vision-2.0

Conversation

@areddish
Copy link
Contributor

@areddish areddish commented May 7, 2018

Description

Regenerate client for v2.0 Swagger
Update tests for new functionality
Update tests to be able to record session when needed

Public submission from private PR#454

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 the commitid of your swagger spec or link to the swagger spec, used to generate the code.
  • The *.csproj and AssemblyInfo.cs files have been updated with the new version of the SDK.

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.

  • Please reference the private repo PR corresponding to this
  • Please pull down latest changes from upstream and run code generation again

@areddish areddish force-pushed the custom-vision-2.0 branch from 1fab8b1 to 02309f0 Compare May 7, 2018 17:48
@areddish
Copy link
Contributor Author

areddish commented May 7, 2018

Private repo PR is https://github.com/Azure/azure-sdk-for-net-pr/pull/832
Swagger PR is Azure/azure-rest-api-specs#3026

Pulled upstream, rebased off it and re-ran code gen, only the metadata changed.

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.

Versioning inconsistent with the private repo PR, please verify

<PackageId>Microsoft.Azure.CognitiveServices.Vision.CustomVision.Prediction</PackageId>
<Description>This client library provides access to the Microsoft Cognitive Services CustomVision Prediction APIs.</Description>
<VersionPrefix>0.9.0-preview</VersionPrefix>
<Version>0.10.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.

There is a diff here with the private repo PR.
Also, PackageReleaseNotes indicate this is not a preview release but the Version does, please verify the intent here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was asked to modify the version and update the preview outside of the private pr. I can send you the info so you can validate.

<PackageId>Microsoft.Azure.CognitiveServices.Vision.CustomVision.Training</PackageId>
<Description>This client library provides access to the Microsoft Cognitive Services CustomVision Training APIs.</Description>
<VersionPrefix>1.0.2-preview</VersionPrefix>
<Version>0.10.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.

Don't think you can go back versions, also differs from the private repo entry, please check the versioning here

ApiKey = TrainingKey
ApiKey = TrainingKey,
#if RECORD_MODE
//BaseUri = new Uri("https://deviris2.azure-api.net/V2.0/Training") //new Uri("http://localhost:8080/api_v2.0/training")
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: remove commented code

ApiKey = TrainingKey,
#if RECORD_MODE
//BaseUri = new Uri("https://deviris2.azure-api.net/V2.0/Training") //new Uri("http://localhost:8080/api_v2.0/training")
BaseUri = new Uri("https://cognitivescusppe.azure-api.net/customvision/v2.0/Training")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need the BaseUri in RECORD_MODE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It depends on how it's recorded, I'm not using it for Prediction so I'm keeping it consistent between Prediction and Training.

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.

LGTM apart from the question

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.

Will merge on CIs passing

@dsgouda dsgouda merged commit 40c6c5d into Azure:psSdkJson6 May 7, 2018
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.

2 participants