jerryshao commented on code in PR #7386: URL: https://github.com/apache/gravitino/pull/7386#discussion_r2155056585
########## api/src/main/java/org/apache/gravitino/exceptions/NoSuchURINameException.java: ########## @@ -0,0 +1,48 @@ +/* + * 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. + */ +package org.apache.gravitino.exceptions; + +import com.google.errorprone.annotations.FormatMethod; + +/** Exception thrown when a URI name of model version is not found. */ +public class NoSuchURINameException extends NotFoundException { Review Comment: How about `NoSuchModelVersionURINameException`? ########## api/src/main/java/org/apache/gravitino/model/ModelCatalog.java: ########## @@ -219,13 +223,70 @@ default boolean modelVersionExists(NameIdentifier ident, String alias) { * @throws NoSuchModelException If the model does not exist. * @throws ModelVersionAliasesAlreadyExistException If the aliases already exist in the model. */ - void linkModelVersion( + default void linkModelVersion( NameIdentifier ident, String uri, String[] aliases, String comment, Map<String, String> properties) - throws NoSuchModelException, ModelVersionAliasesAlreadyExistException; + throws NoSuchModelException, ModelVersionAliasesAlreadyExistException { + linkModelVersion(ident, ImmutableMap.of(URI_NAME_UNKNOWN, uri), aliases, comment, properties); + } + + /** + * Link a new model version to the registered model object. The new model version will be added to + * the model object. If the model object does not exist, it will throw an exception. If the + * version alias already exists in the model, it will throw an exception. + * + * @param ident The name identifier of the model. + * @param uris The names and URIs of the model version artifact. + * @param aliases The aliases of the model version. The aliases should be unique in this model, + * otherwise the {@link ModelVersionAliasesAlreadyExistException} will be thrown. The aliases + * are optional and can be empty. Also, be aware that the alias cannot be a number or a number + * string. + * @param comment The comment of the model version. The comment is optional and can be null. + * @param properties The properties of the model version. The properties are optional and can be + * null or empty. + * @throws NoSuchModelException If the model does not exist. + * @throws ModelVersionAliasesAlreadyExistException If the aliases already exist in the model. + */ + default void linkModelVersion( + NameIdentifier ident, + Map<String, String> uris, + String[] aliases, + String comment, + Map<String, String> properties) + throws NoSuchModelException, ModelVersionAliasesAlreadyExistException { + throw new UnsupportedOperationException("Not supported yet"); + } + + /** + * Get the URI of the model artifact with a specified version number. + * + * @param ident The name identifier of the model. + * @param version The version number of the model. + * @param uriName The name of the URI. If null, the default URI will be used. + * @throws NoSuchModelVersionException If the model version does not exist. + * @return The URI of the model version. + */ + default String getModelUri(NameIdentifier ident, int version, String uriName) Review Comment: How about `getModelVersionUri`? ########## catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelVersionPropertiesMetadata.java: ########## @@ -0,0 +1,43 @@ +/* + * 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. + */ +package org.apache.gravitino.catalog.model; + +import static org.apache.gravitino.model.ModelVersion.PROPERTY_DEFAULT_URI_NAME; + +import com.google.common.collect.ImmutableMap; +import java.util.Map; +import org.apache.gravitino.connector.BasePropertiesMetadata; +import org.apache.gravitino.connector.PropertyEntry; + +public class ModelVersionPropertiesMetadata extends BasePropertiesMetadata { + + @Override + protected Map<String, PropertyEntry<?>> specificPropertyEntries() { + return ImmutableMap.<String, PropertyEntry<?>>builder() + .put( + PROPERTY_DEFAULT_URI_NAME, + PropertyEntry.stringOptionalPropertyEntry( + PROPERTY_DEFAULT_URI_NAME, + "The default URI name for the model version", + false /* immutable */, + null, + false /* hidden */)) + .build(); Review Comment: Do we need to set this property to the model version level, or model level is enough? -- 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]
