jerryshao commented on code in PR #7325:
URL: https://github.com/apache/gravitino/pull/7325#discussion_r2127029793
##########
api/src/main/java/org/apache/gravitino/model/ModelCatalog.java:
##########
@@ -136,6 +136,16 @@ default Model registerModel(
*/
int[] listModelVersions(NameIdentifier ident) throws NoSuchModelException;
+ /**
+ * List all the versions with their information of the register model by
{@link NameIdentifier} in
+ * the catalog.
+ *
+ * @param ident The name identifier of the model.
+ * @return An array of version information of the model.
+ * @throws NoSuchModelException If the model does not exist.
+ */
+ ModelVersion[] listModelVersionsInfo(NameIdentifier ident) throws
NoSuchModelException;
Review Comment:
Can you please update the method name to `listModelVersionInfos`
##########
catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalogOperations.java:
##########
@@ -187,6 +187,23 @@ public int[] listModelVersions(NameIdentifier ident)
throws NoSuchModelException
}
}
+ @Override
+ public ModelVersion[] listModelVersionsInfo(NameIdentifier ident) throws
NoSuchModelException {
+ NameIdentifierUtil.checkModel(ident);
+ Namespace modelVersionNs = NamespaceUtil.toModelVersionNs(ident);
+
+ try {
+ List<ModelVersionEntity> versions =
+ store.list(modelVersionNs, ModelVersionEntity.class,
Entity.EntityType.MODEL_VERSION);
+ return
versions.stream().map(this::toModelVersionImpl).toArray(ModelVersion[]::new);
+
+ } catch (NoSuchEntityException e) {
+ throw new NoSuchModelException(e, "Model %s does not exist", ident);
+ } catch (IOException ioe) {
+ throw new RuntimeException("Failed to list model versions for model " +
ident, ioe);
Review Comment:
"Failed to list model version infos for model ".
##########
common/src/main/java/org/apache/gravitino/dto/responses/ModelVersionListResponse.java:
##########
@@ -31,14 +32,14 @@
public class ModelVersionListResponse extends BaseResponse {
@JsonProperty("versions")
Review Comment:
We're using same json key `versions` for both `modelVersionListResponse` and
`modelVersionNumberListResponse`. I'm afraid some deserialization methods will
be confused when deserializing from JSON, which class should we call?
Also, it may introduce the compatibility issue. For example, if someone uses
REST APIs and they already uses `versions`, not `versions` has another meaning,
how do they differentiate?
I would suggest to use a different name like "infos", what do you think?
--
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]
