hudi-agent commented on code in PR #13147: URL: https://github.com/apache/hudi/pull/13147#discussion_r3408828730
########## hudi-common/src/main/java/org/apache/hudi/common/table/timeline/dto/v2/InstantDTO.java: ########## @@ -0,0 +1,63 @@ +/* + * 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.hudi.common.table.timeline.dto.v2; + +import org.apache.hudi.common.table.timeline.HoodieInstant; +import org.apache.hudi.common.table.timeline.InstantGenerator; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; + +/** + * The data transfer object of instant. + */ +@JsonIgnoreProperties(ignoreUnknown = true) +public class InstantDTO { + + @JsonProperty("action") + public String action; + @JsonProperty("requestTs") + public String requestedTime; + @JsonProperty("completionTs") + public String completionTime; + @JsonProperty("state") + public String state; + + public static InstantDTO fromInstant(HoodieInstant instant) { + if (null == instant) { + return null; + } + + InstantDTO dto = new InstantDTO(); + dto.action = instant.getAction(); + dto.requestedTime = instant.requestedTime(); + dto.completionTime = instant.getCompletionTime(); + dto.state = instant.getState().toString(); + return dto; + } + + public static HoodieInstant toInstant(InstantDTO dto, InstantGenerator factory) { + if (null == dto) { Review Comment: 🤖 nit: could you rename `factory` to `instantGenerator` here? The same type is referred to as `instantGenerator` in `TimelineDTOV2.toTimeline` and in `TimelineHandler`, so calling it `factory` in this one spot breaks the naming convention and makes readers wonder whether it's a different object. <sub><i>- AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub> ########## hudi-timeline-service/src/main/java/org/apache/hudi/timeline/service/handlers/TimelineHandler.java: ########## @@ -46,4 +78,149 @@ public List<InstantDTO> getLastInstant(String basePath) { public TimelineDTO getTimeline(String basePath) { return TimelineDTO.fromTimeline(viewManager.getFileSystemView(basePath).getTimeline()); } + + public TimelineDTOV2 getTimelineV2(String basePath) { + return TimelineDTOV2.fromTimeline(viewManager.getFileSystemView(basePath).getTimeline()); + } + + public Object getInstantDetails(String basePath, String requestedTime, String action, String state) + throws IOException { + HoodieTimeline hoodieTimeline = viewManager.getFileSystemView(basePath).getTimeline(); + try { + InstantGenerator instantGenerator = TimelineLayout.fromVersion( + hoodieTimeline.getTimelineLayoutVersion()).getInstantGenerator(); + HoodieInstant requestedInstant = instantGenerator.createNewInstant( + HoodieInstant.State.valueOf(state), action, requestedTime); + + Object result; + switch (requestedInstant.getAction()) { + case HoodieTimeline.COMMIT_ACTION: + case HoodieTimeline.DELTA_COMMIT_ACTION: + result = hoodieTimeline.readCommitMetadata(requestedInstant); + break; + case HoodieTimeline.CLEAN_ACTION: + result = requestedInstant.isCompleted() + ? hoodieTimeline.readCleanMetadata(requestedInstant) + : hoodieTimeline.readCleanerPlan(requestedInstant); + break; + case HoodieTimeline.ROLLBACK_ACTION: + result = requestedInstant.isCompleted() + ? hoodieTimeline.readRollbackMetadata(requestedInstant) + : hoodieTimeline.readRollbackPlan(requestedInstant); + break; + case HoodieTimeline.RESTORE_ACTION: + result = requestedInstant.isCompleted() + ? hoodieTimeline.readRestoreMetadata(requestedInstant) + : hoodieTimeline.readRestorePlan(requestedInstant); + break; + case HoodieTimeline.SAVEPOINT_ACTION: + result = hoodieTimeline.readSavepointMetadata(requestedInstant); + break; + case HoodieTimeline.COMPACTION_ACTION: + case HoodieTimeline.LOG_COMPACTION_ACTION: + result = requestedInstant.isCompleted() + ? hoodieTimeline.readCommitMetadata(requestedInstant) + : hoodieTimeline.readCompactionPlan(requestedInstant); + break; + case HoodieTimeline.REPLACE_COMMIT_ACTION: + case HoodieTimeline.CLUSTERING_ACTION: + result = requestedInstant.isCompleted() + ? hoodieTimeline.readCommitMetadata(requestedInstant) + : hoodieTimeline.readRequestedReplaceMetadata(requestedInstant); + break; + case HoodieTimeline.INDEXING_ACTION: + result = requestedInstant.isCompleted() + ? hoodieTimeline.readCommitMetadata(requestedInstant) + : hoodieTimeline.readIndexPlan(requestedInstant); + break; + default: + throw new BadRequestResponse("Unsupported action: " + action); + } + + // Avro-generated objects (SpecificRecordBase) cannot be serialized by + // RequestHandler's ObjectMapper+AfterburnerModule due to module access + // restrictions on Avro's internal Schema classes. Convert to plain Maps + // using JsonUtils which accesses fields directly, bypassing getSchema(). + if (result instanceof SpecificRecordBase) { + return JsonUtils.getObjectMapper().convertValue(result, Map.class); + } + return result; + } catch (BadRequestResponse e) { + throw e; + } catch (IllegalArgumentException e) { + throw new BadRequestResponse("Invalid instant state: " + state); + } catch (Exception e) { + log.warn("Failed to read instant details for basePath={}, requestedTime={}, action={}, state={}", + basePath, requestedTime, action, state, e); + throw new HoodieException("Failed to read instant details", e); + } + } + + public Map<String, Object> getTableConfig(String basePath) { + HoodieTableMetaClient metaClient = getOrCreateMetaClient(basePath); + TreeMap<String, String> sorted = new TreeMap<>(); + metaClient.getTableConfig().getProps().forEach((k, v) -> sorted.put(k.toString(), v.toString())); + Map<String, Object> result = new HashMap<>(); + result.put("properties", sorted); + return result; + } + + public Map<String, Object> getSchemaHistory(String basePath, int limit) throws Exception { + HoodieTableMetaClient metaClient = getOrCreateMetaClient(basePath); + + Map<String, Object> result = new HashMap<>(); + + // Get current schema Review Comment: 🤖 nit: `throws Exception` is broader than needed here — every checked exception in the method body is caught internally (either swallowed or rethrown as `HoodieException`/`BadRequestResponse`). Could you narrow this to `throws IOException` or remove the throws clause entirely? As-is, callers see `throws Exception` and have to decide what to catch unnecessarily. <sub><i>- AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub> ########## hudi-timeline-service/src/main/java/org/apache/hudi/timeline/service/RequestHandler.java: ########## @@ -242,6 +258,43 @@ private void registerTimelineAPI() { }, false)); } + /** + * Register v2 Timeline API calls used by the Timeline UI. Gated behind --enable-ui. + */ + private void registerTimelineV2API() { + app.get(RemoteHoodieTableFileSystemView.TIMELINE_V2_URL, new ViewHandler(ctx -> { + metricsRegistry.add("TIMELINE_V2", 1); + TimelineDTOV2 dto = instantHandler.getTimelineV2(getBasePathParam(ctx)); + writeValueAsString(ctx, dto); + }, false)); + + app.get(RemoteHoodieTableFileSystemView.INSTANT_DETAILS_URL, new ViewHandler(ctx -> { + metricsRegistry.add("INSTANT_DETAILS", 1); + Object instantDetails = instantHandler.getInstantDetails(getBasePathParam(ctx), + getInstantParam(ctx), getInstantActionParam(ctx), getInstantStateParam(ctx)); + writeValueAsString(ctx, instantDetails); + }, false)); + + app.get(RemoteHoodieTableFileSystemView.TABLE_CONFIG_V2_URL, new ViewHandler(ctx -> { + metricsRegistry.add("TABLE_CONFIG", 1); + writeValueAsString(ctx, instantHandler.getTableConfig(getBasePathParam(ctx))); + }, false)); + + app.get(RemoteHoodieTableFileSystemView.SCHEMA_HISTORY_V2_URL, new ViewHandler(ctx -> { + metricsRegistry.add("SCHEMA_HISTORY", 1); + int limit; Review Comment: 🤖 nit: the `limit` parsing and validation is done inline here, while every other query param in this class is extracted to a private helper (`getInstantParam`, `getBasePathParam`, etc.). It might be worth pulling this out to something like `getLimitParam(Context ctx)` to keep the pattern consistent and make the lambda body easier to scan. <sub><i>- AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub> -- 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]
