jerryshao commented on code in PR #12627: URL: https://github.com/apache/gravitino/pull/12627#discussion_r4237202809
########## docs/open-api/semantic-models.yaml: ########## @@ -0,0 +1,1590 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +--- + +paths: + + /metalakes/{metalake}/catalogs/{catalog}/schemas/{schema}/semantic-models: + parameters: + - $ref: "./openapi.yaml#/components/parameters/metalake" + - $ref: "./openapi.yaml#/components/parameters/catalog" + - $ref: "./openapi.yaml#/components/parameters/schema" + + get: + tags: + - semantic-model + summary: List Semantic Models + description: >- + Lists the Semantic Models in a schema. Semantic Models are supported only for relational + catalogs. Each returned identifier uses the full metalake.catalog.schema.name namespace. + operationId: listSemanticModels + responses: + "200": + $ref: "#/components/responses/SemanticModelListResponse" + "400": + $ref: "#/components/responses/SemanticModelBadRequestResponse" + "403": + $ref: "#/components/responses/SemanticModelForbiddenResponse" + "404": + $ref: "#/components/responses/SemanticModelParentNotFoundResponse" + "501": + $ref: "#/components/responses/SemanticModelUnsupportedCatalogResponse" + "5XX": + $ref: "./openapi.yaml#/components/responses/ServerErrorResponse" + + post: + tags: + - semantic-model + summary: Create a Semantic Model + description: >- + Creates a schema-scoped Semantic Model in a relational catalog. The complete definition is + validated before the model is stored, including source identifier shape. Source existence, + source columns, and source authorization checks are not yet implemented. + operationId: createSemanticModel + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/SemanticModelCreateRequest" + examples: + SemanticModelCreateRequest: + $ref: "#/components/examples/SemanticModelCreateRequest" + responses: + "200": + $ref: "#/components/responses/SemanticModelResponse" + "400": + $ref: "#/components/responses/SemanticModelBadRequestResponse" + "403": + $ref: "#/components/responses/SemanticModelForbiddenResponse" + "404": + $ref: "#/components/responses/SemanticModelParentNotFoundResponse" + "501": + $ref: "#/components/responses/SemanticModelUnsupportedCatalogResponse" + "409": + $ref: "#/components/responses/SemanticModelAlreadyExistsResponse" + "502": + $ref: "#/components/responses/SemanticModelConnectionFailedResponse" + "5XX": + $ref: "./openapi.yaml#/components/responses/ServerErrorResponse" + + /metalakes/{metalake}/catalogs/{catalog}/schemas/{schema}/semantic-models/ossie: + parameters: + - $ref: "./openapi.yaml#/components/parameters/metalake" + - $ref: "./openapi.yaml#/components/parameters/catalog" + - $ref: "./openapi.yaml#/components/parameters/schema" + + post: + tags: + - semantic-model + summary: Import an Apache Ossie Semantic Model + description: >- + Imports one standalone Apache Ossie document and creates the represented Semantic Model. + The request body is the raw JSON or YAML document, not a JSON-encoded string. Content-Type + selects the parser, and parsing does not fall back to another format. Review Comment: [Important] An absent `Content-Type` is a supported case, but the spec still says it is not. This sentence ("Content-Type selects the parser") and the 415 at lines 141-144 ("Content-Type must be application/json, application/yaml, application/x-yaml, or text/yaml") both read as a mandatory header. The test added in this same commit proves otherwise: `testImportOssieDocumentWithoutContentTypeDefaultsToYaml` (server/src/test/java/org/apache/gravitino/server/web/rest/TestSemanticModelOperations.java:679-724) posts a body with no `Content-Type` and asserts `200` plus `importOssieDocument(namespace, OssieDocument.yaml(yaml))`. The code agrees: `MediaType.APPLICATION_JSON_TYPE.isCompatible(headers.getMediaType())` is false when `getMediaType()` returns null, so the body takes the YAML branch (SemanticModelOperations.java:208-210), which is also what the method Javadoc states - "an absent header defaults to YAML" (SemanticModelOperations.java:171). So the real contract has two halves: header absent -> parsed as YAML; header present but outside the four accepted types -> 415. The spec documents only the second. That matters in two directions - someone writing a client by hand cannot tell that omitting the header is legal, and if the server is later "tightened" to reject a missing header, the spec will make that look like a bug fix rather than the breaking change it would be for existing callers. Suggested fix, one sentence here plus a narrowing of the 415 text: "Content-Type selects the parser; when the header is absent the body is parsed as YAML. Parsing does not fall back to another format." and "Unsupported Media Type - an explicitly supplied Content-Type outside application/json, application/yaml, application/x-yaml and text/yaml". Verified by: reading SemanticModelOperations.java:169-226 and the new test at TestSemanticModelOperations.java:679-724 together with the `doCallRealMethod` wiring at :144-148, and confirming `build (17)` is green on ec3fe00 so that test passes as written. ########## docs/open-api/semantic-models.yaml: ########## @@ -0,0 +1,1590 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +--- + +paths: + + /metalakes/{metalake}/catalogs/{catalog}/schemas/{schema}/semantic-models: + parameters: + - $ref: "./openapi.yaml#/components/parameters/metalake" + - $ref: "./openapi.yaml#/components/parameters/catalog" + - $ref: "./openapi.yaml#/components/parameters/schema" + + get: + tags: + - semantic-model + summary: List Semantic Models + description: >- + Lists the Semantic Models in a schema. Semantic Models are supported only for relational + catalogs. Each returned identifier uses the full metalake.catalog.schema.name namespace. + operationId: listSemanticModels + responses: + "200": + $ref: "#/components/responses/SemanticModelListResponse" + "400": + $ref: "#/components/responses/SemanticModelBadRequestResponse" + "403": + $ref: "#/components/responses/SemanticModelForbiddenResponse" + "404": + $ref: "#/components/responses/SemanticModelParentNotFoundResponse" + "501": + $ref: "#/components/responses/SemanticModelUnsupportedCatalogResponse" + "5XX": + $ref: "./openapi.yaml#/components/responses/ServerErrorResponse" + + post: + tags: + - semantic-model + summary: Create a Semantic Model + description: >- + Creates a schema-scoped Semantic Model in a relational catalog. The complete definition is + validated before the model is stored, including source identifier shape. Source existence, + source columns, and source authorization checks are not yet implemented. + operationId: createSemanticModel + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/SemanticModelCreateRequest" + examples: + SemanticModelCreateRequest: + $ref: "#/components/examples/SemanticModelCreateRequest" + responses: + "200": + $ref: "#/components/responses/SemanticModelResponse" + "400": + $ref: "#/components/responses/SemanticModelBadRequestResponse" + "403": + $ref: "#/components/responses/SemanticModelForbiddenResponse" + "404": + $ref: "#/components/responses/SemanticModelParentNotFoundResponse" + "501": + $ref: "#/components/responses/SemanticModelUnsupportedCatalogResponse" + "409": + $ref: "#/components/responses/SemanticModelAlreadyExistsResponse" + "502": + $ref: "#/components/responses/SemanticModelConnectionFailedResponse" + "5XX": + $ref: "./openapi.yaml#/components/responses/ServerErrorResponse" + + /metalakes/{metalake}/catalogs/{catalog}/schemas/{schema}/semantic-models/ossie: + parameters: + - $ref: "./openapi.yaml#/components/parameters/metalake" + - $ref: "./openapi.yaml#/components/parameters/catalog" + - $ref: "./openapi.yaml#/components/parameters/schema" + + post: + tags: + - semantic-model + summary: Import an Apache Ossie Semantic Model + description: >- + Imports one standalone Apache Ossie document and creates the represented Semantic Model. + The request body is the raw JSON or YAML document, not a JSON-encoded string. Content-Type + selects the parser, and parsing does not fall back to another format. + operationId: importOssieDocument + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/OssieSemanticModelDocument" + examples: + OssieSemanticModelDocument: + $ref: "#/components/examples/OssieSemanticModelDocument" + application/yaml: + schema: + $ref: "#/components/schemas/OssieSemanticModelDocument" + examples: + OssieSemanticModelDocument: + $ref: "#/components/examples/OssieSemanticModelDocument" + application/x-yaml: + schema: + $ref: "#/components/schemas/OssieSemanticModelDocument" + examples: + OssieSemanticModelDocument: + $ref: "#/components/examples/OssieSemanticModelDocument" + text/yaml: + schema: + $ref: "#/components/schemas/OssieSemanticModelDocument" + examples: + OssieSemanticModelDocument: + $ref: "#/components/examples/OssieSemanticModelDocument" + responses: + "200": + $ref: "#/components/responses/SemanticModelResponse" + "400": + $ref: "#/components/responses/SemanticModelBadRequestResponse" + "403": + $ref: "#/components/responses/SemanticModelForbiddenResponse" + "404": + $ref: "#/components/responses/SemanticModelParentNotFoundResponse" + "501": Review Comment: [Nit] `"501"` now sits between `"404"` and `"409"`/`"415"`/`"502"` here, and likewise at lines 77, 205 and 240. The `"405"` it replaced happened to be in ascending position, so the swap left these response maps unsorted, and tooling that preserves document order will render them in this order too. The sibling spec keeps them ascending for the identical set of codes - `statistics.yaml:79-114` has 200, 400, 404, 409, 501, 5xx - so moving each `"501"` below `"502"` would match. Purely cosmetic; fine to leave if you would rather keep the diff minimal. Verified by: `grep -n '"501"' docs/open-api/semantic-models.yaml` (45, 77, 137, 172, 205, 240, 284) and `grep -n '^ "[0-9]' docs/open-api/statistics.yaml` on this branch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
