[Defender (RiskIQ) - EASM] Api version 2023-03-01-preview #23147
Conversation
|
Hi, @thang-bit 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] |
Swagger Validation Report
|
| compared swaggers (via Oad v0.10.4)] | new version | base version |
|---|---|---|
| easm.json | 2023-03-01-preview(877c9dd) | 2022-11-01-preview(main) |
The following breaking changes are detected by comparison with the latest preview version:
Only 30 items are listed, please refer to log for more details.
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️⚠️LintDiff: 14 Warnings warning [Detail]
| compared tags (via openapi-validator v2.1.1) | new version | base version |
|---|---|---|
| 2023-03-01-preview | 2023-03-01-preview(877c9dd) | default(main) |
[must fix]The following errors/warnings are introduced by current PR:
The following errors/warnings exist before current PR submission:
Only 30 items are listed, please refer to log for more details.
| Rule | Message |
|---|---|
| Parameter should have a description. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L112 |
|
| Parameter should have a description. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L627 |
|
| 'PUT' operation 'DiscoveryGroups_Put' should use method name 'Create'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L658 |
|
| Parameter should have a description. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L1031 |
|
| Parameter should have a description. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L1092 |
|
| Parameter should have a description. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L1330 |
|
| 'PUT' operation 'SavedFilters_Put' should use method name 'Create'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L1361 |
|
| Schema should have a description or title. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L1544 |
|
| 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: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L1565 |
|
| Schema should have a description or title. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L1571 |
|
| 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: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L1834 |
|
| Schema should have a description or title. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L1845 |
|
| 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: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L1858 |
|
| 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: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L1868 |
|
| 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: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L1939 |
|
| Schema should have a description or title. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L1982 |
|
| 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: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L2002 |
|
| Schema should have a description or title. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L2071 |
|
| 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: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L2095 |
|
| Schema should have a description or title. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L2113 |
|
| Schema should have a description or title. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L2172 |
|
| Schema should have a description or title. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L2210 |
|
| 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: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L2234 |
|
| Schema should have a description or title. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L2240 |
|
| Schema should have a description or title. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L2423 |
|
| 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: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L2428 |
|
| Schema should have a description or title. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L2513 |
|
| Schema should have a description or title. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L2585 |
|
| Schema should have a description or title. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L2648 |
|
| Schema should have a description or title. Location: Microsoft.Easm/preview/2023-03-01-preview/easm.json#L2763 |
️️✔️Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️ApiReadinessCheck succeeded [Detail] [Expand]
️⚠️~[Staging] ServiceAPIReadinessTest: 0 Warnings warning [Detail]
API Test is not triggered due to precheck failure. Check pipeline log for details.
️️✔️SwaggerAPIView succeeded [Detail] [Expand]
️️✔️CadlAPIView 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).
️️✔️CadlValidation succeeded [Detail] [Expand]
Validation passes for CadlValidation.
️️✔️TypeSpec Validation succeeded [Detail] [Expand]
Validation passes for TypeSpec Validation.
️️✔️PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
|
Swagger pipeline restarted successfully, please wait for status update in this comment. |
Generated ApiView
|
thang-bit
changed the base branch from
riskiq/Microsoft.Easm/2023-03-01-preview
to
main
thang-bit
requested review from
DominikMe and
JeffreyRichter
and removed request for
a team
AzureRestAPISpecReview
added
BreakingChangeReviewRequired
|
Hi @thang-bit, one or multiple breaking change(s) is detected in your PR. Please check out the breaking change(s), and provide business justification in the PR comment and @ PR assignee why you must have these change(s), and how external customer impact can be mitigated. Please ensure to follow breaking change policy to request breaking change review and approval before proceeding swagger PR review. |
|
Hi @thang-bit, one or multiple validation error/warning suppression(s) is detected in your PR. Please follow the Swagger-Suppression-Process to get approval. |
|
Hi @thang-bit, Your PR has some issues. Please fix the CI sequentially by following the order of
|
|
Hi, @thang-bit, For review efficiency consideration, when creating a new api version, it is required to place API specs of the base version in the first commit, and push new version updates into successive commits. You can use OpenAPIHub to initialize the PR for adding a new version. For more details refer to the wiki. Or you could onboard API spec pipeline |
|
Great! @JeffreyRichter This is the summary of things you should review:
Just incase that you want to have more context of the changes and initial feedbacks, this is the link for the record of the last API Review meeting: [RiskIQ - EASM] API Review - Issue _22683-20230321_160403-Meeting Recording.mp4 |
|
I started to watch the video again to refresh myself on the issues and I was not in this meeting at all. So, I think you should get other people to review this as they'd have more context than I do. |
|
@JeffreyRichter I can't change the reviewers on this PR. Who should I contact to change the reviewers? |
|
How did I get added in the first place?
Contact Mark Weitzel
…-- Jeffrey
________________________________
From: thang-bit ***@***.***>
Sent: Tuesday, May 9, 2023 2:11:14 PM
To: Azure/azure-rest-api-specs ***@***.***>
Cc: Jeffrey Richter ***@***.***>; Mention ***@***.***>
Subject: Re: [Azure/azure-rest-api-specs] [Defender (RiskIQ) - EASM] Api version 2023-03-01-preview (PR #23147)
@JeffreyRichter<https://github.com/JeffreyRichter> I can't change the reviewers on this PR. Who should I contact to change the reviewers?
—
Reply to this email directly, view it on GitHub<#23147 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AARLJP2U2P5DPEIL26CVTPLXFKXHFANCNFSM6AAAAAAV536Z4U>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
@JeffreyRichter You are automatically assigned, but I can't change the reviewers. I will contact Mark then |
mikekistler
requested review from
tg-msft and
mikekistler
and removed request for
DominikMe and
JeffreyRichter
|
I have switched reviewers to myself and Ted Glaza. |
specification/riskiq/data-plane/Microsoft.Easm/preview/2023-03-01-preview/easm.json
Show resolved
Hide resolved
specification/riskiq/data-plane/Microsoft.Easm/preview/2023-03-01-preview/easm.json
Show resolved
Hide resolved
specification/riskiq/data-plane/Microsoft.Easm/preview/2023-03-01-preview/easm.json
Show resolved
Hide resolved
specification/riskiq/data-plane/Microsoft.Easm/preview/2023-03-01-preview/easm.json
Show resolved
Hide resolved
specification/riskiq/data-plane/Microsoft.Easm/preview/2023-03-01-preview/easm.json
Show resolved
Hide resolved
|
@mikekistler I have noted the recommendations above to address them before the GA review |
|
@mikekistler, Can we change the other reviewer? I think Ted Glaza is out of office |
specification/riskiq/data-plane/Microsoft.Easm/preview/2023-03-01-preview/easm.json
Show resolved
Hide resolved
specification/riskiq/data-plane/Microsoft.Easm/preview/2023-03-01-preview/easm.json
Show resolved
Hide resolved
specification/riskiq/data-plane/Microsoft.Easm/preview/2023-03-01-preview/easm.json
Show resolved
Hide resolved
specification/riskiq/data-plane/Microsoft.Easm/preview/2023-03-01-preview/easm.json
Show resolved
Hide resolved
|
@weidongxu-microsoft Do these issues also not need to be fixed for previews, but must be fixed for GA? |
No, they probably don't block your preview. |
|
@weidongxu-microsoft I have noted all the comments, can we have an approval on this PR please? |
|
I've mentioned my comment won't block you, but I will leave the sign off to Mike. |
There was a problem hiding this comment.
I discovered a few more issues. The one on the LRO pattern is particularly concerning -- our SDK generators will not give a very good result here. But for preview we can probably live with it. Please add these to the list to fix for your next preview (you should do another preview given the number of changes needed).
specification/riskiq/data-plane/Microsoft.Easm/preview/2023-03-01-preview/easm.json
Show resolved
Hide resolved
specification/riskiq/data-plane/Microsoft.Easm/preview/2023-03-01-preview/easm.json
Show resolved
Hide resolved
specification/riskiq/data-plane/Microsoft.Easm/preview/2023-03-01-preview/easm.json
Show resolved
Hide resolved
|
@mikekistler I have noted all the comments, sending them to the team. However, for some reasons, I am not authorized to merge this PR: |
mikekistler
added
APIStewardshipBoard-SignedOff
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 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 Open API document (swagger) 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