Adding new apis #14910
Adding new apis #14910
Conversation
BulkCollectionsAction QueryApprovedPlans
|
Hi, @MiriBerezin Thanks for your PR. I am workflow bot for review process. Here are some small tips. Any feedback about review process or workflow bot, pls contact swagger and tools team. [email protected] |
|
[Call for Action] To better understand Azure service dev/test scenario, and support Azure service developer better on Swagger and REST API related tests in early phase, please help to fill in with this survey https://aka.ms/SurveyForEarlyPhase. It will take 5 to 10 minutes. If you already complete survey, please neglect this comment. Thanks. |
Swagger Validation Report
|
| Rule | Message |
|---|---|
|
The new version is adding a path that was not found in the old version. New: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L485:5 |
|
The new version is adding a path that was not found in the old version. New: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L530:5 |
️⚠️LintDiff: 7 Warnings warning [Detail]
- Linted configuring files (Based on source branch, openapi-validator v1.10.0 , classic-openapi-validator v1.1.8 )
- Linted configuring files (Based on target branch, openapi-validator v1.10.0 , classic-openapi-validator v1.1.8 )
| Rule | Message |
|---|---|
| Since operation 'PrivateStore_BulkCollectionsAction' response has model definition 'array', it should be of the form '_list'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L543 |
|
| OperationId has a noun that conflicts with one of the model names in definitions section. The model name will be disambiguated to 'PrivateStoreModel'. Consider using the plural form of 'PrivateStore' to avoid this. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L498 |
|
| OperationId has a noun that conflicts with one of the model names in definitions section. The model name will be disambiguated to 'PrivateStoreModel'. Consider using the plural form of 'PrivateStore' to avoid this. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L543 |
|
| Booleans are not descriptive and make them hard to use. Consider using string enums with allowed set of values defined. Property: allSubscriptions Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L2239 |
|
| 'payload' parameter lacks 'description' property. Consider adding a 'description' element. Accurate description is essential for maintaining reference documentation. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L501 |
|
| 'payload' parameter lacks 'description' property. Consider adding a 'description' element. Accurate description is essential for maintaining reference documentation. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L546 |
|
| 'details' model/property lacks 'description' and 'title' property. Consider adding a 'description'/'title' element. Accurate description/title is essential for maintaining reference documentation. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L2212 |
The following errors/warnings exist before current PR submission:
Only 10 items are listed, please refer to log for more details.
| Rule | Message |
|---|---|
|
The schema 'PlanRequesterDetails' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L1660 |
|
The schema 'OperationListResult' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L2354 |
|
The schema 'display' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L2383 |
|
The schema 'error' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L2427 |
|
The schema 'Resource' is considered an object but without a 'type:object', please add the missing 'type:object'. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L2444 |
| Since operation 'PrivateStore_QueryOffers' response has model definition 'array', it should be of the form '_list'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L165 |
|
| Since operation 'PrivateStore_BillingAccounts' response has model definition 'array', it should be of the form '_list'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L201 |
|
| Since operation 'PrivateStoreCollection_TransferOffers' response has model definition 'array', it should be of the form '_list'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L453 |
|
| Since operation 'PrivateStore_GetApprovalRequestsList' response has model definition 'array', it should be of the form '_list'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L765 |
|
| Since operation 'PrivateStore_AdminRequestApprovalsList' response has model definition 'array', it should be of the form '_list'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#L923 |
️️✔️Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️Cross-Version Breaking Changes succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️⚠️[Staging] SDK Track2 Validation: 2 Warnings warning [Detail]
- The following tags are being changed in this PR
| Rule | Message |
|---|---|
| "readme":"marketplace/resource-manager/readme.md", "tag":"package-composite-v2", "details":"The schema 'planDetails-requestDate' has no type or format information whatsoever. Location:\n file:///home/vsts/work/1/azure-rest-api-specs/specification/marketplace/resource-manager/Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#/components/schemas/planDetails-requestDate" |
|
| "readme":"marketplace/resource-manager/readme.md", "tag":"package-2021-06-01", "details":"The schema 'planDetails-requestDate' has no type or format information whatsoever. Location:\n file:///home/vsts/work/1/azure-rest-api-specs/specification/marketplace/resource-manager/Microsoft.Marketplace/stable/2021-06-01/Marketplace.json#/components/schemas/planDetails-requestDate" |
|
| 💬 AutorestCore/Exception | "readme":"marketplace/resource-manager/readme.md", "tag":"package-composite-v2", "details":"> Installing AutoRest extension '@microsoft.azure/openapi-validator' (1.8.0)" |
| 💬 AutorestCore/Exception | "readme":"marketplace/resource-manager/readme.md", "tag":"package-composite-v2", "details":"> Installed AutoRest extension '@microsoft.azure/openapi-validator' (1.8.0->1.8.0)" |
The following errors/warnings exist before current PR submission:
|:speech_balloon: AutorestCore/Exception|"readme":"marketplace/resource-manager/readme.md",
"tag":"package-composite-v2",
"details":"> Loading AutoRest extension '@autorest/modelerfour' (4.15.456->4.15.456)"|
|:speech_balloon: AutorestCore/Exception|"readme":"marketplace/resource-manager/readme.md",
"tag":"package-2021-06-01",
"details":"> Loading AutoRest extension '@microsoft.azure/openapi-validator' (1.8.0->1.8.0)"|
|:speech_balloon: AutorestCore/Exception|"readme":"marketplace/resource-manager/readme.md",
"tag":"package-2021-06-01",
"details":"> Loading AutoRest extension '@autorest/modelerfour' (4.15.456->4.15.456)"|
️️✔️[Staging] PrettierCheck succeeded [Detail] [Expand]
Validation passes for PrettierCheck.
️️✔️[Staging] SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️[Staging] Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
Swagger Generation Artifacts
|
|
NewApiVersionRequired reason: |
@MiriBerezin please follow the tips in the comment |
|
@jianyexi I posted a request for breaking changes. Can explain here as well: We are still in development mode and we need these two APIs. https://msazure.visualstudio.com/One/_workitems/edit/10195585 Please let me know if there are further steps I need to take |
|
Do not merge, thanks |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
We are ready to merge after review. Thanks! |
jianyexi
added
the
WaitForARMFeedback
|
Please go through and complete the checklist in the PR description |
pilor
added
the
ARMChangesRequested
|
@pilor, done |
|
2021-06-01 is not in the RP manifest so updates to this version are OK |
| } | ||
| } | ||
| }, | ||
| "QueryApprovedPlansResponse": { |
There was a problem hiding this comment.
This will be easier to consume if its an array of plan objects with planId as a property in each object. Why does it need to be a dictionary with dynamic keys?
There was a problem hiding this comment.
We want to enforce the plan id uniqueness in the result
There was a problem hiding this comment.
You control the response so you can enforce uniqueness regardless of the response schema. If that is the only reason please use a strict schema instead of dynamic keys
There was a problem hiding this comment.
Fixed to list of objects
| } | ||
| } | ||
| }, | ||
| "/providers/Microsoft.Marketplace/privateStores/{privateStoreId}/bulkCollectionsAction": { |
There was a problem hiding this comment.
Why does this need a bulk operation instead of performing the operation on each collection individually? Bulk operations are problematic because they may perform actions the user did not have RBAC rights to perform (since the operation is not performed directly on the scope it affects) and they don't expose in the activity log everything they are modifying. It also affects downstream notifications of changes, etc...
There was a problem hiding this comment.
Currently, we don't have RBAC per collection. It is on post Ni roadmap.
Once we get there, we have 2 options ( on the RP side):
- Check RBAC for each collection and fail the action if there is a least one collection that is not authorized.
- Check RBAC for each collection and activate only for the allowed ones. In this case, the response will contain the details for the ones that passed and the ones that failed. The current response already supports this scenario.
There was a problem hiding this comment.
Can you clarify why a bulk operation is needed? RBAC is only one of the side effects mentioned above
There was a problem hiding this comment.
We have a business requirement to perform an action delete/enable/disable for all collections in a click.
On the server side, we have a transaction(DB stored procedure) performing that operation. If we don't do that in bulk we get conflicts and high-performance impact and not acceptable latency.
Thus the API must be a bulk operation.
pilor
added
ARMSignedOff
|
@jianyexi we got arm sign-off we would like to merge to master |
|
No deploy api-version 2021-06-01. Approve this breakingChange. |
ruowan
added
the
Approved-BreakingChange
* Adding new apis: BulkCollectionsAction QueryApprovedPlans * Fix queryApprovedOffers api * fix prettier check * Fix queryApprovedPlans API * Fix queryApprovedPlans response to list instead if disctionary
BulkCollectionsAction
QueryApprovedPlans
MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.
Changelog
Please ensure to add changelog with this PR by answering the following questions.
Contribution checklist:
If any further question about AME onboarding or validation tools, please view the FAQ.
ARM API Review Checklist
[V] Ensure to check this box if one of the following scenarios meet updates in the PR, so that label “WaitForARMFeedback” will be added automatically to involve ARM API Review. Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs, all “removals” and “adding a new property” no more require ARM API review.
[V] Please ensure you've reviewed following guidelines including ARM resource provider contract and REST guidelines. Estimated time (4 hours). This is required before you can request review from ARM API Review board.
If you are blocked on ARM review and want to get the PR merged with urgency, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
Breaking Change Review Checklist
If there are following updates in the PR, ensure to request an approval from Breaking Change Review Board as defined in the Breaking Change Policy.
Action: to initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Addition details on the process and office hours are on the Breaking change Wiki.
Please follow the link to find more details on PR review process.