[Hub Generated] Review request for Azure.Analytics.Purview.Catalog to add version preview/2023-02-01-preview #25164
Conversation
…review/2022-08-01-preview to version 2023-02-01-preview
|
Hi, @yifan-zhou922! 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 |
|---|---|---|
| purviewcatalog.json | 2023-02-01-preview(b21a12d) | 2022-08-01-preview(main) |
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️⚠️LintDiff: 15 Warnings warning [Detail]
| compared tags (via openapi-validator v2.1.4) | new version | base version |
|---|---|---|
| package-preview-2023-02 | package-preview-2023-02(b21a12d) | 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 |
|---|---|
|
The host parameter must be called 'endpoint'. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L12 |
|
The host parameter must be typed 'type 'string', format 'url''. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L12 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Entity' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L304 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Entity' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L610 |
|
Inline/anonymous models must not be used, instead define a schema with a model name in the 'definitions' section and refer to it. This allows operations to share the models. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L974 |
|
Inline/anonymous models must not be used, instead define a schema with a model name in the 'definitions' section and refer to it. This allows operations to share the models. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L1026 |
|
Inline/anonymous models must not be used, instead define a schema with a model name in the 'definitions' section and refer to it. This allows operations to share the models. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L1075 |
|
Inline/anonymous models must not be used, instead define a schema with a model name in the 'definitions' section and refer to it. This allows operations to share the models. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L1124 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L1559 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L1598 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L1645 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L1684 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L1715 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L1755 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L1785 |
|
Inline/anonymous models must not be used, instead define a schema with a model name in the 'definitions' section and refer to it. This allows operations to share the models. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L1851 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L1923 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L1968 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L2017 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L2060 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L2090 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L2136 |
|
Inline/anonymous models must not be used, instead define a schema with a model name in the 'definitions' section and refer to it. This allows operations to share the models. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L2381 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L2407 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L2438 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L2481 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L2511 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L2557 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L2603 |
|
Per the Noun_Verb convention for Operation Ids, the noun 'Glossary' should not appear after the underscore. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. Location: Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json#L2639 |
️️✔️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
|
…023-02-01-preview' of ssh://github.com/Azure/azure-rest-api-specs into yifan-zhou922-purview-Azure.Analytics.Purview.Catalog-2023-02-01-preview
… add version preview/2022-08-01-preview (#25150) * add content-type header parameter * polish glossary related api document * update * update
|
Hi @yifan-zhou922! 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. |
|
Hi @mikekistler, we are ready for API review. |
| { | ||
| "$ref": "#/parameters/Content-Type" | ||
| }, |
There was a problem hiding this comment.
Header parameter "Content-Type" should not be defined explicitly.
| { | |
| "$ref": "#/parameters/Content-Type" | |
| }, |
There was a problem hiding this comment.
Thanks. We want to let users understand how to set the content-type when calling our REST APIs directly as we noticed that some customers did not set the correct content-type when using postman to call this API. So, we want to provide more information in the public document in the same way as the storage service.
Here is the document from Storage service listing the requests headers including content-length and content-type. https://learn.microsoft.com/en-us/rest/api/storageservices/put-blob?tabs=azure-ad#request-headers-all-blob-types
Shall we also mark the content-type as optional? Also, the current documentation does not support showing formData parameters. So, we are trying to find a way to let customers know how to correctly use this API. Here is the work item tracked by Reference team. https://dev.azure.com/ceapex/Engineering/_workitems/edit/766112
There was a problem hiding this comment.
In OpenAPI, the set of allowed content types is defined by the "consumes" field of the operation. In this case, only one content type is allowed, "multipart/form-data". To avoid the potential for ambiguous/conflicting API descriptions, OpenAPI does not allow Content-type to be defined explicitly.
If
inis"header"and thenamefield is"Accept","Content-Type"or"Authorization", the parameter definition SHALL be ignored.
Given that only one content-type is allowed, I think the service should simply make this the default when content-type is not specified and then users don't even need to think about it.
If you really think you need to document that the only content-type they can pass is "multipart/form-data" you could add a statement to that effect in the operation description.
There was a problem hiding this comment.
Understand. Content-type header is removed. We will use another way to document this information. Thanks
| { | ||
| "$ref": "#/parameters/Content-Type" | ||
| }, |
There was a problem hiding this comment.
Header parameter "Content-Type" should not be defined explicitly.
| { | |
| "$ref": "#/parameters/Content-Type" | |
| }, |
| "tags": [ | ||
| "Glossary" | ||
| ], | ||
| "description": "Update terms in bulk.", |
There was a problem hiding this comment.
Does this PUT completely replace all terms, or just add to the set of terms? If it is the latter, then the method should be POST rather than PUT.
There was a problem hiding this comment.
Yes. It will replace all terms. So we use PUT instead of POST.
There was a problem hiding this comment.
It looks like the Apache API implements this as a POST and not PUT:
https://atlas.apache.org/api/v2/ui/index.html#/
Why the change to use PUT here?
There was a problem hiding this comment.
Atlas has a create term API using POST and an update term API using PUT.
This API is bulk update term API, so we use PUT instead of POST.
...ew/data-plane/Azure.Analytics.Purview.Catalog/preview/2023-02-01-preview/purviewcatalog.json
Show resolved
Hide resolved
| "type": "array", | ||
| "description": "An array of glossary terms or an empty list.", | ||
| "items": { | ||
| "$ref": "#/definitions/AtlasGlossaryTerm" | ||
| } |
There was a problem hiding this comment.
Response should not be a bare array. The response should be an object with a top-level property that is an array.
If this operation is borrowed from open-source, then this is allowed. Otherwise it should be changed as stated.
There was a problem hiding this comment.
Thanks @mikekistler. This response pattern is also following the atlas behavior.
Comments addressed. Please help review the changes. Thanks again.
Next Steps to Merge |
| "nickName": { | ||
| "type": "string", | ||
| "description": "The nick name of the term." | ||
| }, | ||
| "hierarchyInfo": { | ||
| "type": "array", | ||
| "description": "The hierarchy information of the term.", | ||
| "items": { | ||
| "$ref": "#/definitions/PurviewObjectId" | ||
| } | ||
| }, |
There was a problem hiding this comment.
AtlasGlossaryTerm is used in API /atlas/v2/glossary/term/{termGuid}, so I assume it be atlas data type.
Now we add nickName and hierarchyInfo into it. Are they atlas data, or our data?
PurviewObjectId looks like our augmentation to AtlasObjectId.
| "properties": { | ||
| "type": "object", | ||
| "additionalProperties": {} | ||
| } |
There was a problem hiding this comment.
If this is our data, do we really don't know what properties there is?
There was a problem hiding this comment.
Thanks. These are all our data. As PurviewObjectId is shared in many scenarios, it will contain different keys in various scenarios.
mikekistler
added
the
APIStewardshipBoard-SignedOff
|
Hi @tjprescott could you please help review and signoff the changes. Many thanks. |
tjprescott
deleted the
yifan-zhou922-purview-Azure.Analytics.Purview.Catalog-2023-02-01-preview
branch
|
Swagger pipeline restarted successfully, please wait for status update in this comment. |
This is a PR generated at OpenAPI Hub. You can view your work branch via this link.
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.
These APIs have been reviewed here: [Microsoft Purview ] API Review #24004
Diff: https://openapihub.azure-devex-tools.com/tools/diff/eyJzb3VyY2VVcmwiOiJodHRwczovL2dpdGh1Yi5jb20vQXp1cmUvYXp1cmUtcmVzdC1hcGktc3BlY3MvYmxvYi95aWZhbi16aG91OTIyLXB1cnZpZXctQXp1cmUuQW5hbHl0aWNzLlB1cnZpZXcuQ2F0YWxvZy0yMDIzLTAyLTAxLXByZXZpZXcvc3BlY2lmaWNhdGlvbi9wdXJ2aWV3L2RhdGEtcGxhbmUvQXp1cmUuQW5hbHl0aWNzLlB1cnZpZXcuQ2F0YWxvZy9wcmV2aWV3LzIwMjItMDgtMDEtcHJldmlldy9wdXJ2aWV3Y2F0YWxvZy5qc29uIiwidGFyZ2V0VXJsIjoiaHR0cHM6Ly9naXRodWIuY29tL0F6dXJlL2F6dXJlLXJlc3QtYXBpLXNwZWNzL2Jsb2IveWlmYW4temhvdTkyMi1wdXJ2aWV3LUF6dXJlLkFuYWx5dGljcy5QdXJ2aWV3LkNhdGFsb2ctMjAyMy0wMi0wMS1wcmV2aWV3L3NwZWNpZmljYXRpb24vcHVydmlldy9kYXRhLXBsYW5lL0F6dXJlLkFuYWx5dGljcy5QdXJ2aWV3LkNhdGFsb2cvcHJldmlldy8yMDIzLTAyLTAxLXByZXZpZXcvcHVydmlld2NhdGFsb2cuanNvbiJ9
❔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