[Language] Restructure Language swagger files
Data Plane API Specification Update Pull Request
[!TIP] Overwhelmed by all this guidance? See the
Getting helpsection at the bottom of this PR description.
PR review workflow diagram
Please understand this diagram before proceeding. It explains how to get your PR approved & merged.
API Info: The Basics
Most of the information about your service should be captured in the issue that serves as your API Spec engagement record.
- Link to API Spec engagement record issue:
Is this review for (select one):
- [ ] a private preview
- [ ] a public preview
- [x] GA release
Change Scope
This section will help us focus on the specific parts of your API that are new or have been modified.
Please share a link to the design document for the new APIs, a link to the previous API Spec document (if applicable), and the root paths that have been updated.
- Design Document:
- Previous API Spec Doc:
- Updated paths:
Viewing API changes
For convenient view of the API changes made by this PR, refer to the URLs provided in the table
in the Generated ApiView comment added to this PR. You can use ApiView to show API versions diff.
Suppressing failures
If one or multiple validation error/warning suppression(s) is detected in your PR, please follow the Swagger-Suppression-Process to get approval.
❔Got questions? Need additional info?? We are here to help!
Contact us!
The Azure API Review Board is dedicated to helping you create amazing APIs. You can read about our mission and learn more about our process on our wiki.
- 💬 Teams Channel
Click here for links to tools, specs, guidelines & other good stuff
Tooling
- Open API validation tools were run on this PR. Go here to see how to fix errors
- Spectral Linting
Guidelines & Specifications
Helpful Links
Getting help
- First, please carefully read through this PR description, from top to bottom.
- If you don't have permissions to remove or add labels to the PR, request
write accessper aka.ms/azsdk/access#request-access-to-rest-api-or-sdk-repositories - To understand what you must do next to merge this PR, see the
Next Steps to Mergecomment. It will appear within few minutes of submitting this PR and will continue to be up-to-date with current PR state. - For guidance on fixing this PR CI check failures, see the hyperlinks provided in given failure and https://aka.ms/ci-fix.
- If the PR CI checks appear to be stuck in
queuedstate, please add a comment with contents/azp run. This should result in a new comment denoting aPR validation pipelinehas started and the checks should be updated after few minutes. - If the help provided by the previous points is not enough, post to https://aka.ms/azsdk/support/specreview-channel and link to this PR.
This comment is a remnant of a now-deprecated bot framework. It is being left behind only to point at the new comment that is updated by our new actions-based framework.
PR validation pipeline restarted successfully. If there is ApiView generated, it will be updated in this comment.
@heaths to give some context, since we already have separate api release schedules for Language, our generated API docs are not reflecting the correct API versions for each capability.
So during the API review for the new API version we were asked to get this fixed. In an attempt to fix, that I submitted this PR to re-organize the swagger files according to the api grouping we will release. I will also update the related documentation/SDKs in a separate PR.
The organization we are proposing (after discussion with our internal team):
Language/AnalyzeConversations - Typespec project Language.Conversations
Language/AnalyzeText - Typespce project Language.AnalyzeText
Language/AnalyzeDocuments - Typespec project Language.AnalyzeDocuments
Language/AnalyzeConversations.Authoring (as suggested) Typespec project Language.AnalyzeConversations-authoring
Language/AnalyzeText.Authoring (as suggested) - Typespec project Language.AnalyzeText-authoring
Language/QuestionAnswering - not in typespec yet.
Language/QuestionAnswering.Authoring - not in type spec yet.
Adding @JeffreyRichter, @mikekistler @allenjzhang for their review/comments
This PR looks pretty GREAT to me! Then the tsp files will go in each service directory, right? For example, the TSP files fpr AnalyzeConversations will fo in the /specificaiotn/cognitiveservices/data-plane/Langauge/AnalyzeConversations folder and each time this gets converted to swagger, the resulting swagger will go in the preview or stable folder with another subdir matching the api-version, right?
Can we get rid of the preview folder that is immediately under the Language directory? This folder currently has 2021-110-01-preview and 2023-04-15-preview subdirs in it.
@JeffreyRichter , yes, I am updating the typespecs as well to point to the correct swagger directory in this next iteration.
The preview folder under Language shows up as there were 2 random files which were part of the preview folders, which I am assuming should not have been merged. They show up as removed as they are not moved anywhere. Rather they are deleted.
- oav
- readme
@heaths @mikekistler @allenjzhang,
As discussed in the last language API review here is the PR to separate the individual language APIs to enable independent release cadence. As expected there are a lot of test and automation failures, Can you please help review and approve this? Once this PR is merged we will start to release the new beta SDKs. Also in case we need to update anywhere else, please share the pointers.
PR validation pipeline can not start as the pull request is not merged or mergeable - most likely it has merge conflicts.
PR validation pipeline can not start as the pull request is not merged or mergeable - most likely it has merge conflicts.
API Change Check
APIView identified API level changes in this PR and created the following API reviews
| Language | API Review for Package |
|---|---|
| TypeSpec | Language.Conversations.Authoring |
| Swagger | AnomalyDetector-LanguageAnalyzeConversationsAuthoring |
| TypeSpec | Language.Conversations |
| TypeSpec | Language.AnalyzeDocuments |
| TypeSpec | Language.Text |
| TypeSpec | Language.Text.Authoring |
| TypeSpec | Language.QuestionAnswering |
| TypeSpec | Language.QuestionAnswering.Authoring |
This has been open for almost 2 years and suddenly there's more activity. @bidisha-c please let me know if/when you're expecting another review. Until then, I highly recommend making it a draft so we know it's not ready to review. There are clearly a lot of validation errors that need to be worked out yet.
This has been open for almost 2 years and suddenly there's more activity. @bidisha-c please let me know if/when you're expecting another review. Until then, I highly recommend making it a draft so we know it's not ready to review. There are clearly a lot of validation errors that need to be worked out yet.
@Heath we were blocked on this PR as we had many preview APIs which were not removed. As a result this restructure caused multiple validation failures. However, now we have deprecated all old previews and retiring them . I have merged the pr to remove all old preview APIs last week https://github.com/Azure/azure-rest-api-specs/pull/31781
So I have resumed this PR and it is ready for review
We want to priotize and merge this ASAP
@heaths, @mikekistler, @JeffreyRichter
I am looking for advise on how to fix the errors and merge this PR?
For AnalyzeText and Conversations typespec project, the change is to update the Typespec location to create the swaggers and update individual READMEs.
But for the rest of them, we have a few missing old GA API versions in Typespec:
QuestionAnsweringandQuestionANswering.Authoring- we are missing2021-10-01GA API in Typespec. It was manually moved to the new structureConversationsandConversations.Authoring-2022-05-01API is missing in Typespec. Also manually moved to the new folder structure
Now this is causing bunch of failures in validation. How do we handle this?
If the old versions' swaggers are available and just aren't in the TypeSpecs, I'm in favor of bypassing those validation errors; however, @mikekistler and @JeffreyRichter are the ones on the breaking change board. This is just my $0.02, not that that's worth what it used to be.
@mikekistler I am still not able to merge. It complains about missing Typespec for the older GA APIs missing for QuestionAnswering and Conversations.Authoring and Analyze-Text.Authoring. How do we handle these?
I've overridden that Avocado error, but I'm concerned about the LintDiff error since the endpoint should be {endpoint} and not {endpoint}/language. Is generated code handling that correctly on the TypeSpecs, or is generated code still using the swaggers and you don't know? The emitters might make certain assumptions, right or wrong. If clients emitting based on the TypeSpec are working, it may be fine; though, I worry about other customers who might still use the swaggers.
@heath I did not follow your comment here but the endpoint is unchanged in the swagger.
But for the lint diffs errors are only for the 2 old API versions which are missing either in Authoring or QuestionAnswering.
@mikekistler @johanste I don't remember who is supposed to approve suppressions. Is anyone on the REST API board?
Not sure about the other blocking issue mentioned in "sign off requirements".
@lmazuel is this you?
Adding @mikeharder and @weshaggard , it seems some of those issues are specific to v1 to v2 migration.
@bidisha-c: Please update your branch with the latest changes from main and resolve conflicts, so we can see the latest check results.
Next Steps to Merge
✅ All automated merging requirements have been met! To get your PR merged, see aka.ms/azsdk/specreview/merge.Comment generated by summarize-checks workflow run.
@mikeharder I am still not able to merge
@JeffreyRichter, @qiaozha: If this spec is being reorganized, should it convert directly to folder structure v2? It's currently using v1.
TypeSpecRequirement: https://github.com/Azure/azure-rest-api-specs/wiki/TypeSpec-Requirement#renaming-specs
LintDiff: https://github.com/Azure/azure-rest-api-specs/wiki/Swagger-LintDiff#adding-scoped-suppressions
@JeffreyRichter, @qiaozha: If this spec is being reorganized, should it convert directly to folder structure v2? It's currently using v1.
There're two problems here that is violating the folder structure v2.
- It's not unifying the TypeSpec and swagger into the same service folder, like ./specification/cognitiveservices/data-plane/{serviceName}
- In folder structure v2, I don't think we support service group concept, which means the Language folder in ./specification/cognitiveservices/data-plane/Language/{serviceName} should not be supported. If you have to have a service group to indicate that they are somewhat related, I'd suggest to use Language as the serviceName prefix instead.
@JeffreyRichter could you double confirm here? I see you have approved this in last October, which should probably happened before this folder structure v2 guideline being finalized.
@JeffreyRichter, @qiaozha: If this spec is being reorganized, should it convert directly to folder structure v2? It's currently using v1.
There're two problems here that is violating the folder structure v2.
- It's not unifying the TypeSpec and swagger into the same service folder, like ./specification/cognitiveservices/data-plane/{serviceName}
- In folder structure v2, I don't think we support service group concept, which means the Language folder in ./specification/cognitiveservices/data-plane/Language/{serviceName} should not be supported. If you have to have a service group to indicate that they are somewhat related, I'd suggest to use Language as the serviceName prefix instead.
@JeffreyRichter could you double confirm here? I see you have approved this in last October, which should probably happened before this folder structure v2 guideline being finalized.
This is our current folder structure proposed in this PR is
Typespec:
/specifications/cognitiveservices/Language.Conversations/
/specifications/cognitiveservices/Language.AnalyzeText/
...
data-plane
/specifications/cognitiveservices/data-plane/Language/Language.Conversations
/specifications/cognitiveservices/data-plane/Language/Language.AnalyzeText
...
When you mentioned to move under ./specification/cognitiveservices/data-plane/{serviceName}, does that mean we move the typespec and swagger under same structure?
Typespec:
/specifications/cognitiveservices/Language/Language.Conversations/*.tsp files (all typespec files)
/specifications/cognitiveservices/Language/Language.Conversations/stable/**/*.json files (swaggers)
The folder structure is supposed to be /specifications/cognitiveservices/data-plane/<public-facing Azure service name> So, how do these services appear to customers in the doc TOC? I'm sure we don't call it "Azure Language Language Conversations". Is it Azure Language Conversations? if so, then we want: /specifications/cognitiveservices/data-plane/LanguageConversations /specifications/cognitiveservices/data-plane/LanguageAnalyzeText
How do customers discover these services and learn their name when then then use for docs and SDK discovery? This is how we also do repo folder discovery.
Azure Language Conversations
The unified service is named as "Azure AI Language" https://learn.microsoft.com/en-us/azure/ai-services/language-service/overview
- Under this we have Conversations, Analyze-Text, QuestionAnswering etc.
When you mentioned to move under ./specification/cognitiveservices/data-plane/{serviceName}, does that mean we move the typespec and swagger under same structure?
@bidisha-c: Yes. You can use our sample as an example:
https://github.com/Azure/azure-rest-api-specs/tree/main/specification/widget/data-plane/WidgetAnalytics
Note you may need to update paths in your tspconfig.yaml after moving the file.
@JeffreyRichter @mikeharder @qiaozha the restructuring has now been done following the v2 folder structure. Now the Language services have structured as
/specification/cognitiveservices/data-plane/LanguageAnalyzeConversations /specification/cognitiveservices/data-plane/LanguageAnalyzeConversationsAuthoring /specification/cognitiveservices/data-plane/LanguageAnalyzeDocuments /specification/cognitiveservices/data-plane/LanguageAnalyzeText /specification/cognitiveservices/data-plane/LanguageAnalyzeTextAuthoring /specification/cognitiveservices/data-plane/LanguageQuestionAnswering /specification/cognitiveservices/data-plane/LanguageQuestionAnsweringAuthoring