Repository navigation
Conversation
FrankChen021
left a comment
There was a problem hiding this comment.
🟡 Changes recommended
The new deserialization path still erases Kafka-specific offset types: real active-task reports will deserialize nested offsets with untyped keys and values, so offset lookups and round-trip equality are incorrect. Preserve the Kafka generic types before merging.
Reviewed 5 of 5 changed files; findings in 1 file.
Validation: git diff --check bb50f29b1d792c7b82e48dd24435be5966e8b269..HEAD passed. No builds or tests were run.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 0 |
| P3 | 0 |
| Total | 1 |
This is an automated review by Codex GPT-5.6-Luna(max)
After addressing the findings or replying to the comments, you can request another review from me to trigger a new automated review.
| @JsonProperty("ignored2") SupervisorStateManager.State detailedState, | ||
| @JsonProperty("recentErrors") List<SupervisorStateManager.ExceptionEvent> recentErrors | ||
| @JsonProperty("recentErrors") List<SupervisorStateManager.ExceptionEvent> recentErrors, | ||
| @JsonProperty("activeTasks") @Nullable List<TaskReportData> activeTasks, |
There was a problem hiding this comment.
[P1] Raw task lists erase Kafka offset types
Finding: Because these creator parameters are raw List, Jackson constructs each task without the KafkaTopicPartition/Long type bindings. When a real status contains non-empty startingOffsets, currentOffsets, or lag maps, their keys are deserialized as strings and offset values may be untyped numeric values, so lookups by KafkaTopicPartition can return null or casts can fail even though the task list itself is present.
Suggestion: Preserve the concrete TaskReportData<KafkaTopicPartition, Long> types through the payload constructors and extend the round-trip test with non-null partition-offset and lag maps.
Description
Support deserializing activeTasks/publishingTasks in KafkaSupervisorReportPayload.
This PR has: