Add areaOfInterest and detectObject to CV API #4771

mirzamo · 2018-12-04T21:27:09Z

This PR has been approved in the private repo. As we are about to release these features to public, I'm sending this PR.
https://github.com/Azure/azure-rest-api-specs-pr/pull/607

Latest improvements:

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

Contribution checklist:

I have reviewed the documentation for the workflow.
Validation tools were run on swagger spec(s) and have all been fixed in this PR.
The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
adding/removing APIs.
adding/removing properties.
adding/removing API-version.
adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
Please follow the link to find more details on API review process.

* Replace the current swagger with an auto-generated one. * Add example files for areaOfInterest and detectObject. * Fix example validation errors for analyze and describe. * A few fixes for generating java sdk

openapi-portal-comment · 2018-12-04T21:27:10Z

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

azuresdkci · 2018-12-04T21:27:27Z

Can one of the admins verify this patch?

AutorestCI · 2018-12-04T21:27:55Z

Automation for azure-sdk-for-python

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

AutorestCI · 2018-12-04T21:28:31Z

Automation for azure-sdk-for-ruby

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

AutorestCI · 2018-12-04T21:28:37Z

Automation for azure-sdk-for-java

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

AutorestCI · 2018-12-04T21:28:39Z

Automation for azure-sdk-for-node

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

AutorestCI · 2018-12-04T21:29:23Z

Automation for azure-sdk-for-go

A PR has been created for you:
Azure/azure-sdk-for-go#3596

AutorestCI · 2018-12-04T21:58:48Z

Automation for azure-sdk-for-js

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

yangyuan · 2018-12-04T23:45:42Z

Hi @mirzamo, earlier today I had a meeting with Hui and Cha today.
We will need API Review Board to review the swaggers before we proceed.

specification/cognitiveservices/data-plane/ComputerVision/stable/v2.0/ComputerVision.json

jianghaolu · 2018-12-05T18:50:45Z

specification/cognitiveservices/data-plane/ComputerVision/stable/v2.0/Ocr.json

+            "InvalidModel",
+            "CancelledRequest",
+            "NotSupportedVisualFeature",
+            "FailedToProcess",


This seems to be a breaking change for your API - current SDKs will not be able to handle these new enums. Only enums with "modelAsString" set to true are allowed to add new enum values.

Can you help clarify? If the service returns one of these new values in the JSON, aren't current clients screwed anyway?

@jianghaolu Any comments?

Yes That's what I mean by saying the rest API is going to break the clients.

aren't current clients screwed anyway

This doesn't justify breaking the clients after breaking the APIs. If this list is supposed to grow you should mark "modelAsString" to true in this PR. This PR is bringing breaking changes anyways (unfortunately) but there will be no more in the future.

I changed it to true to prevent future breaking changes. Also in here.

mirzamo · 2018-12-07T00:33:32Z

@yangyuan I addressed all of your comments.
Regarding the API review board, I sent over the swagger and sdk, but the comments are either related to autorest which we don't have any control over it, or naming. We cannot make any changes to the names at this time since the SDKs are already shipped.

yangyuan · 2018-12-07T05:43:29Z

@mirzamo
Can you cc me the email thread? (alias yuanyang)
For existing issues (not related to this PR) we can totally skip them and fix them in next version, but need PM's awareness.

yangyuan

Look good from my side.
Also passes typo check.

yangyuan · 2018-12-07T06:37:04Z

...rvices/data-plane/ComputerVision/stable/v2.0/examples/SuccessfulDetectObjectsWithStream.json

+        "metadata": {
+          "width": 100,
+          "height": 100,
+          "format": "Jpeg"


Just an idea, wouldn't jpeg or JPEG be better?

I agree, but for all the of other services it's Jpeg and I think it's better to keep them consistent.

Just curious, what services you are referring to? I searched before and didn't find anyother..

Analyze
Tag
Describe

Oh you mean other operations, I thought you mean other services

specification/cognitiveservices/data-plane/ComputerVision/stable/v2.0/ComputerVision.json

… ComputerVisonErrors enum

* Add areaOfInterest and detectObject * Replace the current swagger with an auto-generated one. * Add example files for areaOfInterest and detectObject. * Fix example validation errors for analyze and describe. * A few fixes for generating java sdk * Addressing commets: reorder some keys * fix formatting * Changing modelAsString parameter to ture to be able to add new values ComputerVisonErrors enum

Add areaOfInterest and detectObject

0bc8927

* Replace the current swagger with an auto-generated one. * Add example files for areaOfInterest and detectObject. * Fix example validation errors for analyze and describe. * A few fixes for generating java sdk

mirzamo requested a review from yangyuan as a code owner December 4, 2018 21:27

azuresdkci assigned jianghaolu Dec 4, 2018

azuresdkci requested a review from jianghaolu December 4, 2018 21:27

AutorestCI mentioned this pull request Dec 4, 2018

[AutoPR cognitiveservices/data-plane/ComputerVision] Add areaOfInterest and detectObject to CV API Azure/azure-sdk-for-python#3997

Merged

AutorestCI mentioned this pull request Dec 4, 2018

[AutoPR cognitiveservices/data-plane/ComputerVision] Add areaOfInterest and detectObject to CV API Azure/azure-sdk-for-ruby#2044

Merged

AutorestCI mentioned this pull request Dec 4, 2018

[AutoPR cognitiveservices/data-plane/ComputerVision] Add areaOfInterest and detectObject to CV API Azure/azure-sdk-for-java#2735

Merged

AutorestCI mentioned this pull request Dec 4, 2018

[AutoPR cognitiveservices/data-plane/ComputerVision] Add areaOfInterest and detectObject to CV API Azure/azure-sdk-for-node#4348

Merged

AutorestCI mentioned this pull request Dec 4, 2018

[AutoPR cognitiveservices/data-plane/ComputerVision] Add areaOfInterest and detectObject to CV API Azure/azure-sdk-for-go#3596

Merged

AutorestCI mentioned this pull request Dec 4, 2018

[AutoPR cognitiveservices/data-plane/ComputerVision] Add areaOfInterest and detectObject to CV API Azure/azure-sdk-for-js#731

Merged

yangyuan reviewed Dec 4, 2018

View reviewed changes

specification/cognitiveservices/data-plane/ComputerVision/stable/v2.0/ComputerVision.json Outdated Show resolved Hide resolved

yangyuan reviewed Dec 4, 2018

View reviewed changes

specification/cognitiveservices/data-plane/ComputerVision/stable/v2.0/ComputerVision.json Outdated Show resolved Hide resolved

yangyuan reviewed Dec 4, 2018

View reviewed changes

specification/cognitiveservices/data-plane/ComputerVision/stable/v2.0/ComputerVision.json Outdated Show resolved Hide resolved

jianghaolu suggested changes Dec 5, 2018

View reviewed changes

Addressing commets: reorder some keys

49e8819

mirzamo force-pushed the aoi-od branch from 25b32f2 to 49e8819 Compare December 6, 2018 01:12

yangyuan approved these changes Dec 7, 2018

View reviewed changes

fix formatting

c5c4e81

mirzamo force-pushed the aoi-od branch from a8d3100 to c5c4e81 Compare December 7, 2018 23:08

Changing modelAsString parameter to ture to be able to add new values…

13a861a

… ComputerVisonErrors enum

jianghaolu approved these changes Dec 11, 2018

View reviewed changes

jianghaolu merged commit 6d123b9 into Azure:master Dec 11, 2018

mirzamo mentioned this pull request Dec 12, 2018

Add AOI and OD features Azure/azure-sdk-for-net#5078

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add areaOfInterest and detectObject to CV API #4771

Add areaOfInterest and detectObject to CV API #4771

mirzamo commented Dec 4, 2018 •

edited

Loading

openapi-portal-comment bot commented Dec 4, 2018

azuresdkci commented Dec 4, 2018

AutorestCI commented Dec 4, 2018 •

edited

Loading

AutorestCI commented Dec 4, 2018 •

edited

Loading

AutorestCI commented Dec 4, 2018 •

edited

Loading

AutorestCI commented Dec 4, 2018 •

edited

Loading

AutorestCI commented Dec 4, 2018 •

edited

Loading

AutorestCI commented Dec 4, 2018 •

edited

Loading

yangyuan commented Dec 4, 2018

jianghaolu Dec 5, 2018

cthrash Dec 5, 2018

mirzamo Dec 7, 2018

jianghaolu Dec 7, 2018

jianghaolu Dec 7, 2018

mirzamo Dec 8, 2018

mirzamo commented Dec 7, 2018

yangyuan commented Dec 7, 2018

yangyuan left a comment

yangyuan Dec 7, 2018

mirzamo Dec 7, 2018

yangyuan Dec 8, 2018

mirzamo Dec 8, 2018

yangyuan Dec 8, 2018

Add areaOfInterest and detectObject to CV API #4771

Add areaOfInterest and detectObject to CV API #4771

Conversation

mirzamo commented Dec 4, 2018 • edited Loading

Latest improvements:

Contribution checklist:

ARM API Review Checklist

openapi-portal-comment bot commented Dec 4, 2018

azuresdkci commented Dec 4, 2018

AutorestCI commented Dec 4, 2018 • edited Loading

Automation for azure-sdk-for-python

AutorestCI commented Dec 4, 2018 • edited Loading

Automation for azure-sdk-for-ruby

AutorestCI commented Dec 4, 2018 • edited Loading

Automation for azure-sdk-for-java

AutorestCI commented Dec 4, 2018 • edited Loading

Automation for azure-sdk-for-node

AutorestCI commented Dec 4, 2018 • edited Loading

Automation for azure-sdk-for-go

AutorestCI commented Dec 4, 2018 • edited Loading

Automation for azure-sdk-for-js

yangyuan commented Dec 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mirzamo commented Dec 7, 2018

yangyuan commented Dec 7, 2018

yangyuan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mirzamo commented Dec 4, 2018 •

edited

Loading

AutorestCI commented Dec 4, 2018 •

edited

Loading

AutorestCI commented Dec 4, 2018 •

edited

Loading

AutorestCI commented Dec 4, 2018 •

edited

Loading

AutorestCI commented Dec 4, 2018 •

edited

Loading

AutorestCI commented Dec 4, 2018 •

edited

Loading

AutorestCI commented Dec 4, 2018 •

edited

Loading