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

[Cognitive Services - Document Translator] REST API Review for Document Batch Translator API v1.1 #16794

Closed
czubair opened this issue Nov 17, 2021 · 1 comment · Fixed by #16705
Assignees
Labels
APIStewardshipBoard-SignedOff The Azure API Stewardship team has reviewed and approved the changes.

Comments

@czubair
Copy link
Contributor

czubair commented Nov 17, 2021

Swagger PR: #16705

Service name: Document Batch Translator

Key contact for this review: alcheng;faanis

Are you in the PST time zone? If not, what time zone are you in? PST;Eastern European Time (Egypt = UTC+2)

Is this a new API or the first API for the service? No

Link to previous API version
https://github.com/Azure/azure-rest-api-specs/tree/main/specification/cognitiveservices/data-plane/TranslatorText/stable/v1.0

Has your service already been deployed?
No

Description of the material to be reviewed/changed:
https://microsoft-my.sharepoint.com/:w:/p/faanis/ES4dpXMEuc5IjGeTXMA6iwMBqstA6zvTV09h2XgZT8LI9A?e=gI8OiY

@czubair czubair added the APIStewardshipBoard-ReviewRequested This should be reviewed by the Azure API Stewardship team in partnership with the service team. label Nov 17, 2021
@czubair czubair assigned johanste and unassigned mikekistler Nov 17, 2021
@czubair czubair linked a pull request Nov 17, 2021 that will close this issue
26 tasks
@czubair czubair removed a link to a pull request Nov 17, 2021
26 tasks
@czubair czubair linked a pull request Nov 17, 2021 that will close this issue
26 tasks
@markweitzel
Copy link
Member

markweitzel commented Dec 1, 2021

API Stewardship Meeting 1-Dec-21

The main motivation for adding this field is for discovery of the @failedDocumentStatus endpoint.

  • Recommendation: Fixt the documentation first. This would avoid having to update this.
    • The examples in the quickstarts don't leverage the SDKs--shows raw code that don't show the pattern we'd like to promote.
  • We should look at following the long running operations pattern more closely. Operation Location will have the status of the batch. Within that, there will be a status for each of the documents that are getting translated.

The standard pattern for "jump off to find the actual results for the LRO" is to have a resourceLocation attribute with the link. It would have the full set of results, though - whereas the current proposed failed results location filters on only the failed docs.

For the Word conversion API

  • Consider restructuring this to have an extensible enum, with "WORD" and "NONE" as the only options. "NONE" would be the default. This will add a bit of "future proofing"
  • The options for converting the document to word should be at the target level. This could also be done at the input/job level. You should make the call based on what they see their customers doing most so long as we could version in more flexibility over time.

Misc

  • Add a default error. Try to follow the structure in the guidelines if possible.

References

@markweitzel markweitzel added APIStewardship-ChangesRequested <valid label in PR review process>add this label when assignee request changes after review and removed APIStewardshipBoard-ReviewRequested This should be reviewed by the Azure API Stewardship team in partnership with the service team. labels Dec 1, 2021
@markweitzel markweitzel changed the title [Document Translator] REST API Review for Document Batch Translator API v1.1 [Cognitive Services - Document Translator] REST API Review for Document Batch Translator API v1.1 Jan 21, 2022
@markweitzel markweitzel added APIStewardshipBoard-SignedOff The Azure API Stewardship team has reviewed and approved the changes. and removed APIStewardship-ChangesRequested <valid label in PR review process>add this label when assignee request changes after review labels Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APIStewardshipBoard-SignedOff The Azure API Stewardship team has reviewed and approved the changes.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants