-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add API Version for new Purview Data Plane API 2023-12-01-preview #27333
Add API Version for new Purview Data Plane API 2023-12-01-preview #27333
Conversation
…review/2023-10-01-preview to version 2023-12-01-preview
Add bulk edit asset example
This reverts commit 2bfb90e.
Revert Test
Next Steps to MergeNext steps that must be taken to merge this PR:
|
Swagger Validation Report
|
Rule | Message |
---|---|
Runtime Exception |
"new":"https://github.com/Azure/azure-rest-api-specs/blob/fdd97d50be1db4f7e3c9173dcc56d5ba121ba9b2/specification/purview/data-plane/Azure.Analytics.Purview.DataMap/preview/2023-12-01-preview/purviewdatamap.json", "old":"https://github.com/Azure/azure-rest-api-specs/blob/main/specification/purview/data-plane/Azure.Analytics.Purview.DataMap/stable/2023-09-01/purviewdatamap.json", "details":"Breaking change detector (OAD) invoked AutoRest. AutoRest threw a runtime error. First 6 lines of stack trace follow, indexed. First line should contain AutoRest command line invocation details. Second line should contain the main message reported by AutoRest. ==================== 1: Command failed: node "/mnt/vss/_work/_tasks/AzureApiValidation_5654d05d-82c1-48da-ad8f-161b817f6d41/0.0.90/common/temp/node_modules/.pnpm/@Azure+oad@0.10.5/node_modules/autorest/dist/app.js" --v2 --input-file=specification/purview/data-plane/Azure.Analytics.Purview.DataMap/preview/2023-12-01-preview/purviewdatamap.json --output-artifact=swagger-document.json --output-artifact=swagger-document.map --output-file=new --output-folder=/tmp -------------------- 2: ERROR: Schema violation: No enum match for: operation-location -------------------- 3: - file:///mnt/vss/_work/1/azure-rest-api-specs/specification/purview/data-plane/Azure.Analytics.Purview.DataMap/preview/2023-12-01-preview/purviewdatamap.json:1710:10 ($.paths["/entity/bulk/delete"].delete["x-ms-long-running-operation-options"]["final-state-via"]) -------------------- 4: FATAL: swagger-document/individual/schema-validator - FAILED -------------------- 5: FATAL: Error: [OperationAbortedException] Error occurred. Exiting. -------------------- 6: Process() cancelled due to exception : [OperationAbortedException] Error occurred. Exiting. --------------------" |
The following breaking changes are detected by comparison with the latest preview version:
Rule | Message |
---|---|
Runtime Exception |
"new":"https://github.com/Azure/azure-rest-api-specs/blob/fdd97d50be1db4f7e3c9173dcc56d5ba121ba9b2/specification/purview/data-plane/Azure.Analytics.Purview.DataMap/preview/2023-12-01-preview/purviewdatamap.json", "old":"https://github.com/Azure/azure-rest-api-specs/blob/main/specification/purview/data-plane/Azure.Analytics.Purview.DataMap/preview/2023-10-01-preview/purviewdatamap.json", "details":"Breaking change detector (OAD) invoked AutoRest. AutoRest threw a runtime error. First 6 lines of stack trace follow, indexed. First line should contain AutoRest command line invocation details. Second line should contain the main message reported by AutoRest. ==================== 1: Command failed: node "/mnt/vss/_work/_tasks/AzureApiValidation_5654d05d-82c1-48da-ad8f-161b817f6d41/0.0.90/common/temp/node_modules/.pnpm/@Azure+oad@0.10.5/node_modules/autorest/dist/app.js" --v2 --input-file=/mnt/vss/_work/1/cross-version-c93b354fd9c14905bb574a8834c4d69b/specification/purview/data-plane/Azure.Analytics.Purview.DataMap/preview/2023-10-01-preview/purviewdatamap.json --output-artifact=swagger-document.json --output-artifact=swagger-document.map --output-file=old --output-folder=/tmp -------------------- 2: ERROR: Schema violation: No enum match for: operation-location -------------------- 3: - file:///mnt/vss/_work/1/cross-version-c93b354fd9c14905bb574a8834c4d69b/specification/purview/data-plane/Azure.Analytics.Purview.DataMap/preview/2023-10-01-preview/purviewdatamap.json:1620:10 ($.paths["/entity/bulk/delete"].delete["x-ms-long-running-operation-options"]["final-state-via"]) -------------------- 4: FATAL: swagger-document/individual/schema-validator - FAILED -------------------- 5: FATAL: Error: [OperationAbortedException] Error occurred. Exiting. -------------------- 6: Process() cancelled due to exception : [OperationAbortedException] Error occurred. Exiting. --------------------" |
️️✔️
CredScan succeeded [Detail] [Expand]
There is no credential detected.
️❌
LintDiff: 0 Errors, 41 Warnings failed [Detail]
Compared specs (v2.2.0) | new version | base version |
---|---|---|
package-preview-2023-12 | package-preview-2023-12(fdd97d5) | default(main) |
[must fix]The following errors/warnings are introduced by current PR:
Only 30 items are listed, please refer to log for more details.
The following errors/warnings exist before current PR submission:
Only 30 items are listed, please refer to log for more details.
️️✔️
Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️
SwaggerAPIView succeeded [Detail] [Expand]
️️✔️
TypeSpecAPIView succeeded [Detail] [Expand]
️❌
ModelValidation: 2 Errors, 0 Warnings failed [Detail]
Rule | Message |
---|---|
OBJECT_ADDITIONAL_PROPERTIES |
Additional properties not allowed: businessAttributeDefs Url: Azure.Analytics.Purview.DataMap/preview/2023-12-01-preview/purviewdatamap.json#L5059:23 ExampleUrl: preview/2023-12-01-preview/examples/Type_List_With_FullName.json#L244:38 |
OBJECT_ADDITIONAL_PROPERTIES |
Additional properties not allowed: businessAttributeDefs Url: Azure.Analytics.Purview.DataMap/preview/2023-12-01-preview/purviewdatamap.json#L5059:23 ExampleUrl: preview/2023-12-01-preview/examples/Type_List_With_FullName.json#L244:38 |
️️✔️
SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️
PoliCheck succeeded [Detail] [Expand]
Validation passed for PoliCheck.
️️✔️
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
|
Hi @xuyan3! 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. |
…01-preview' of https://github.com/xuyan3/azure-rest-api-specs into xuyan3-purview-Azure.Analytics.Purview.DataMap-2023-12-01-preview
…ure-rest-api-specs into xuyan3-purview-Azure.Analytics.Purview.DataMap-2023-12-01-preview
…01-preview' of https://github.com/xuyan3/azure-rest-api-specs into xuyan3-purview-Azure.Analytics.Purview.DataMap-2023-12-01-preview
…01-preview' of https://github.com/xuyan3/azure-rest-api-specs into xuyan3-purview-Azure.Analytics.Purview.DataMap-2023-12-01-preview
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 left a few comments in the APIView, but to recap the review meeting:
- Discuss amongst your team how to better map settings and typeDefs. Reviewers had trouble understanding it so customers likely will as well. It seems these are two separate resources collections, so it might make sense to have /typeDefs (or similar, which you already have) and /settings, each with their own CRUD operations as appropriate. It might mean that customers using settings first have to create settings to refer to them in typeDefs, but that order of operations is not uncommon across many Azure services.
- In general, use PATCH to create or update (we typically call these operationIds "CreateOrUpdate_{ResourceType}", in fact).
- For icons, there should be one collection - /types/icons - on which you have CRUD operations:
- /types/icons
- GET: List_Icons
- POST: Create_Icon - only if the customer doesn't control the ID/name, which I believe you said they can control it so you probably won't have this
- /types/icons/{name}
- PATCH: CreateOrUpdate_Icon
- GET: Get_Icon
- DELETE: Delete_Icon
- /types/icons
- For the type namespace changes, just always include the new
fullTypeName
without having customers pass?withFullTypeName=true
. Only support that query parameter where needed i.e., where you return an array of eithertypeName
orfullTypeName
.
Thanks Heaths for help with API review. For Icon APIs, we updated paths using plural. But after internal discussions, we decided to go with PUT/POST instead of PATCH. Mainly because icon is a rather simple scenario and we want to keep users clear for whether they are creating or replacing an icon, without worrying about overwrite. |
Hi, @xuyan3. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove |
Hi, @xuyan3. The PR will be closed since the PR has no update for 28 days. If you still need the PR review to proceed, please reopen it and @ mention PR assignee. |
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.
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.
Click here for links to tools, specs, guidelines & other good stuff
Tooling
Guidelines & Specifications
Helpful Links
Checks stuck in `queued` state?
If the PR CI checks appear to be stuck in `queued` state, please add a comment with contents `/azp run`. This should result in a new comment denoting a `PR validation pipeline` has started and the checks should be updated after few minutes.