wenjin272 commented on code in PR #950:
URL: https://github.com/apache/flink-agents/pull/950#discussion_r3725525928
##########
api/src/main/java/org/apache/flink/agents/api/Event.java:
##########
@@ -125,7 +149,19 @@ public static Event fromEvent(Event event) {
* @throws IOException if JSON parsing fails or the 'type' field is
missing or empty
*/
public static Event fromJson(String json) throws IOException {
- return MAPPER.readValue(json, Event.class);
+ Event event = MAPPER.readValue(json, Event.class);
+ for (Map.Entry<String, Object> entry :
event.getAttachments().entrySet()) {
Review Comment:
**[P1] Preserve `MemoryRef` when restoring `ActionState`**
Thanks for adding JSON support for `MemoryRef`. This conversion only runs
when `Event.fromJson()` is called explicitly. Durable recovery instead
deserializes the enclosing `ActionState` directly through `ActionStateSerde`,
so attachment values declared as `Object` are restored as `LinkedHashMap`
rather than `MemoryRef`. `loadEventAttachments()` then skips them, and the
recovered action receives the reference-shaped map instead of the original
payload.
A possible implementation sketch would be to bind a content deserializer
directly to the `attachments` values:
```java
@JsonCreator
public Event(
// ...
@JsonProperty("attachments")
@JsonDeserialize(contentUsing = AttachmentValueDeserializer.class)
Map<String, Object> attachments) {
// ...
}
static final class AttachmentValueDeserializer extends
JsonDeserializer<Object> {
@Override
public Object deserialize(JsonParser p, DeserializationContext ctxt)
throws IOException {
JsonNode node = p.getCodec().readTree(p);
if (node.isObject()
&& "memory_ref".equals(node.path("@type").asText())) {
return p.getCodec().treeToValue(node, MemoryRef.class);
}
return p.getCodec().treeToValue(node, Object.class);
}
}
```
The `MemoryRef` serializer would emit a language-neutral discriminator, for
example `{"@type": "memory_ref", "memory_type": "sensory", "path": "..."}`.
Merely adding the discriminator to the JSON is not sufficient: because the
declared map value type is still `Object`, Jackson also needs this
property-level deserialization hook (or an equivalent central hook) to select
`MemoryRef.Deserializer`. This should use the same discriminator on the Python
side, and an `ActionStateSerde` round-trip test should verify that the restored
attachment is still a `MemoryRef`.
--
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]