-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Adding deprecation messages to preview versions 2022-08-17 & 2022-09-25 #25194
Adding deprecation messages to preview versions 2022-08-17 & 2022-09-25 #25194
Conversation
2022-08-17 & 2022-09-25 (according to 45554387 item)
Hi, @galkeinan-microsoft! Thank you for your pull request. To help get your PR merged: Generated ApiView comment added to this PR. You can use ApiView to show API versions diff. |
Swagger Validation Report
|
compared swaggers (via Oad v0.10.4)] | new version | base version |
---|---|---|
search.json | 2022-08-17-preview(846eee4) | 2022-08-17-preview(main) |
search.json | 2022-09-25-preview(846eee4) | 2022-09-25-preview(main) |
️️✔️
Breaking Change(Cross-Version) succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️
CredScan succeeded [Detail] [Expand]
There is no credential detected.
️⚠️
LintDiff: 0 Warnings warning [Detail]
compared tags (via openapi-validator v2.1.4) | new version | base version |
---|---|---|
package-2022-08-17-preview | package-2022-08-17-preview(846eee4) | package-2022-08-17-preview(main) |
package-2022-09-25-preview | package-2022-09-25-preview(846eee4) | package-2022-09-25-preview(main) |
The following errors/warnings exist before current PR submission:
Only 30 items are listed, please refer to log for more details.
Rule | Message |
---|---|
ValidFormats |
'decimal' is not a known format. Location: Search/preview/2022-08-17-preview/search.json#L961 |
ValidFormats |
'decimal' is not a known format. Location: Search/preview/2022-08-17-preview/search.json#L966 |
The error property in the error response schema should be required.Location: Search/preview/2022-08-17-preview/search.json#L515 |
|
Error schema should define code and message properties as required.Location: Search/preview/2022-08-17-preview/search.json#L515 |
|
Property should have a defined type. Location: Search/preview/2022-08-17-preview/search.json#L682 |
|
Property should have a defined type. Location: Search/preview/2022-08-17-preview/search.json#L697 |
|
Property should have a defined type. Location: Search/preview/2022-08-17-preview/search.json#L705 |
|
Property should have a defined type. Location: Search/preview/2022-08-17-preview/search.json#L717 |
|
Property should have a defined type. Location: Search/preview/2022-08-17-preview/search.json#L982 |
|
Property should have a defined type. Location: Search/preview/2022-08-17-preview/search.json#L1002 |
|
Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Search/preview/2022-08-17-preview/search.json#L1021 |
|
Property should have a defined type. Location: Search/preview/2022-08-17-preview/search.json#L1068 |
|
Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Search/preview/2022-08-17-preview/search.json#L1076 |
|
Property should have a defined type. Location: Search/preview/2022-08-17-preview/search.json#L1122 |
|
Property should have a defined type. Location: Search/preview/2022-08-17-preview/search.json#L1166 |
|
A required parameter should not specify a default value. Location: Search/preview/2022-09-25-preview/search.json#L501 |
|
The error property in the error response schema should be required.Location: Search/preview/2022-09-25-preview/search.json#L514 |
|
Error schema should define code and message properties as required.Location: Search/preview/2022-09-25-preview/search.json#L514 |
|
A required parameter should not specify a default value. Location: Search/preview/2022-09-25-preview/search.json#L942 |
|
Avoid the use of x-nullable. Location: Search/preview/2022-09-25-preview/search.json#L947 |
|
The error property in the error response schema should be required.Location: Search/preview/2022-09-25-preview/search.json#L955 |
|
Error schema should define code and message properties as required.Location: Search/preview/2022-09-25-preview/search.json#L955 |
|
Since operation response has model definition in array type, it should be of the form '_list'. Location: Search/preview/2022-09-25-preview/search.json#L973 |
|
Based on the response model schema, operation 'Suggestions_GetProducts' might be pageable. Consider adding the x-ms-pageable extension. Location: Search/preview/2022-09-25-preview/search.json#L973 |
|
Operation might be pageable. Consider adding the x-ms-pageable extension. Location: Search/preview/2022-09-25-preview/search.json#L973 |
|
Avoid the use of x-nullable. Location: Search/preview/2022-09-25-preview/search.json#L1319 |
|
A required parameter should not specify a default value. Location: Search/preview/2022-09-25-preview/search.json#L1336 |
|
The error property in the error response schema should be required.Location: Search/preview/2022-09-25-preview/search.json#L1348 |
|
Error schema should define code and message properties as required.Location: Search/preview/2022-09-25-preview/search.json#L1348 |
|
Property should have a defined type. Location: Search/preview/2022-09-25-preview/search.json#L1555 |
️️✔️
Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️
SwaggerAPIView succeeded [Detail] [Expand]
️️✔️
TypeSpecAPIView succeeded [Detail] [Expand]
️️✔️
ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️
SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️
PoliCheck succeeded [Detail] [Expand]
Validation passed for PoliCheck.
️️✔️
PrettierCheck succeeded [Detail] [Expand]
Validation passes for PrettierCheck.
️️✔️
SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️
Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
️️✔️
PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
️️✔️
Automated merging requirements met succeeded [Detail] [Expand]
Swagger Generation Artifacts
|
Generated ApiView
|
please review but don't approve yet. |
}, | ||
"azure-deprecating": { | ||
"type": "string", | ||
"description": "Search API version 2022-08-17 will retire on Jan. 31, 2024. After this date, this API version will no longer be supported. To continue using our services, it is essential to migrate to the new Search API preview version 2023-01-01.\nFor detailed guidance, please refer to the updated API documentation, available at: https://learn.microsoft.com/en-us/rest/api/marketplacecatalog/2023-01-01-preview/search/get?tabs=HTTP\nIf you have any questions or require assistance during the transition, our support team is ready to help. Feel free to contact us at MKPL_Platform_API_DL@microsoft.com" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume it is 2022-08-17-preview
get retired, not 2022-08-17
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, will fix it.
@weidongxu-microsoft
by the way, why the "openapi-pipeline-app / Swagger ApiDocPreview" seems stuck (I cant see the preview of the docs)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It happens sometimes that the CI got stuck. Usually re-run would work...
I see CI is all green now. I've approved the PR. Let me know when you need it merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @weidongxu-microsoft,
I don't see any visual notification in the apiPreview informimg of deprecation (where is the text informing of deprecation I added?).
How can I add such visual deprecation notice? red banner on top with the text I added (see this item created by Product Manager: https://microsoft.visualstudio.com/DefaultCollection/OSGS/_workitems/edit/45554387)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The azure-deprecating
here has nothing to do with deprecation information on API doc page.
This is providing a runtime azure-deprecating
header to your user, so they would see it in logger etc.
https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md#deprecating-behavior-notification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so what is the common way to add such notification on deprecation? that will be visual to users- red banner on top with the text I added (see this item created by Product Manager: https://microsoft.visualstudio.com/DefaultCollection/OSGS/_workitems/edit/45554387)
Thanks for the quick response!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not familiar with deprecation procedure. Please check with your PM for deprecation procedure.
If you want to make it clear on MSLearn that certain version will be deprecated, maybe update the page https://learn.microsoft.com/en-us/rest/api/marketplacecatalog/ (it should correspond to a md file in github repo), though I've no idea about red banner. <-- this is only a suggestion, as mentioned, I am not familiar with the procedure
Hi @galkeinan-microsoft! Your PR has some issues. Please fix the CI issues, if present, in following order:
If you need further help, please reach out on the Teams channel aka.ms/azsdk/support/specreview-channel. |
https://github.com/galkeinan-microsoft/azure-rest-api-specs into galkeinan/add-deprecation-message-to-old-api-versions
Swagger Generation Artifacts
|
Swagger Validation Report
|
Swagger Validation Report
|
Swagger Generation Artifacts
|
Next Steps to Merge |
Automatic PR validation restarted. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛. |
Automatic PR validation started. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛. |
Automatic PR validation restarted. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛. |
https://github.com/galkeinan-microsoft/azure-rest-api-specs into galkeinan/add-deprecation-message-to-old-api-versions
Automatic PR validation restarted. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛. |
Automatic PR validation restarted. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛. |
Hi @weidongxu-microsoft Why the preview html created by "openapi-pipeline-app / Swagger ApiDocPreview" shows only 2022-09-25-preview changes? what about 2022-08-17 changes - where can I see them? |
Automatic PR validation restarted. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛. |
Automatic PR validation started. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛. |
Please complete this PR if seems ok. |
"azure-deprecating": { | ||
"type": "string", | ||
"description": "Deprecation Message" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you confirm that your backend indeed return azure-deprecating
header with text in runtime, like https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md#deprecating-behavior-notification ?
Give a sample if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, should be in prod by end of this week (we are rolling a new version with the new header).
the response contains header "azure-deprecating"with value set to:
"API versions 2022-09-25-preview & 2022-08-17-preview will retire on Jan. 31, 2024. After this date, this API version will no longer be supported. To continue using our services, it is essential to migrate to the new Search API preview version 2023-01-01.For detailed guidance, please refer to the updated API documentation, available at: https://learn.microsoft.com/en-us/rest/api/marketplacecatalog/2023-01-01-preview/search/get?tabs=HTTP If you have any questions or require assistance during the transition, please contact us at MKPL_Platform_API_DL@microsoft.com"
please approve this PR if seems ok, so we can publish it as soon as possible.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The format seems not same as the guideline (especially the date).
https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md#deprecating-behavior-notification
Also please make sure you've met the requirement there (breaking change review and azure update).
@JeffreyRichter @mikekistler for awareness on deprecation.
- marketplacecatalog deprecates 2022-08-17-preview and 2022-09-25-preview.
- Latest preview is 2023-01-01-preview. It is more than 2 month old (so I assume it meets the policy, of 90 days overlap window).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mikekistler,
The communication is ready and about to be sent (our Product Manager is working with Shashank Vineet on it).
I added the header 'azure-deprecating' with wrong formatted message as @weidongxu-microsoft mention, but understood this header is not mandatory and dont have to match the communication (the same message is about to be sent in communication).
the 'wrong formatted' header should be available in prod in the next 2 days.
I can change the "azure-deprecating" value we return to match the format you sent, but it will reach prod only next week.
please let me know how to proceed, we would like to publish this PR as soon as possible (it contains a red banner on top informing users on the deprecation, although it is not required)
I can delete the header (from both this PR and from our API), or djust its value.
But we really want to publish this PR since it contains a red banner on top with a notification of the deprecation.
In case you want me to change the header's value, please share the exact message that you consider as valid, since we dont have a url as shown in the example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mikekistler Let us know if you have opinion here.
I assume azure-deprecating
header does not need to be an exact copy of your communication (via email or other) to customer.
azure-deprecating
header in Swagger and runtime is intended to notify user that calling the API, giving them context that the plan/schedule/time of the deprecation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The content in azure-deprecating
does not need to be an exact copy of the communication. The intent of the url
part of the header value is to avoid duplicating the communication by instead referencing it.
So I recommend simply fixing the formatting as needed (I'm not sure I grasp the problem with the format, but I don't think I need to.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mikekistler @weidongxu-microsoft
Thanks for your help.
Decided with @weidongxu-microsoft that we will complete this PR and publish the documentation as is, and next week "azure-depreacting" message will be fixed.
since we dont have the url part of the message, the only change will be in the date format (in bold).
wrong formatted message:
"API versions 2022-09-25-preview & 2022-08-17-preview will retire on Jan. 31, 2024. After this date, this API version will no longer be supported. To continue using our services, it is essential to migrate to the new Search API preview version 2023-01-01.For detailed guidance, please refer to the updated API documentation, available at: https://learn.microsoft.com/en-us/rest/api/marketplacecatalog/2023-01-01-preview/search/get?tabs=HTTP If you have any questions or require assistance during the transition, please contact us at MKPL_Platform_API_DL@microsoft.com"
new message:
"API versions 2022-09-25-preview & 2022-08-17-preview will retire on 2024-01-31. After this date, this API version will no longer be supported. To continue using our services, it is essential to migrate to the new Search API preview version 2023-01-01. For detailed guidance, please refer to the updated API documentation, available at: https://learn.microsoft.com/rest/api/marketplacecatalog/2023-01-01-preview/search/get?tabs=HTTP If you have any questions or require assistance during the transition, please contact us at MKPL_Platform_API_DL@microsoft.com"
@weidongxu-microsoft please confirm the new message, so I can start rolling the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK for me. Note that you have the recommended api-version in message, and the retire will be on 2024-01-31. If you have new api-version as recommendation before 2024-01-31, you will need to update the message here (that is the reason why URL is recommended), to point to newer api-version.
After 2024-01-31 this api-version will be gone and you won't need to update.
Swagger pipeline restarted successfully, please wait for status update in this comment. |
2022-08-17 & 2022-09-25 (according to 45554387 item)
Data Plane API - Pull Request
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.
Is this review for (select one):
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.
❔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.
Click here for links to tools, specs, guidelines & other good stuff
Tooling
Guidelines & Specifications
Helpful Links