joeyutong commented on code in PR #1030:
URL: https://github.com/apache/flink-agents/pull/1030#discussion_r3840837958
##########
api/src/main/java/org/apache/flink/agents/api/context/MemoryUpdate.java:
##########
@@ -32,17 +32,36 @@ public class MemoryUpdate implements Serializable {
private final String path;
private final Object value;
+ private final boolean objectCreation;
/**
- * Creates a new MemoryUpdate instance.
+ * Creates a new MemoryUpdate instance describing a value write.
*
* @param path the absolute path of the data in Short-Term Memory.
* @param value the new value to set at the specified path.
*/
+ public MemoryUpdate(String path, Object value) {
+ this(path, value, false);
+ }
+
+ /**
+ * Creates a new MemoryUpdate instance.
+ *
+ * @param path the absolute path of the data in Short-Term Memory.
+ * @param value the new value to set at the specified path; always null
when {@code
+ * objectCreation} is true.
+ * @param objectCreation true if this update records the creation of a
nested object rather than
+ * a value write. Absent in records written before this field existed,
in which case Jackson
+ * defaults it to false, preserving their original replay behavior.
+ */
@JsonCreator
- public MemoryUpdate(@JsonProperty("path") String path,
@JsonProperty("value") Object value) {
+ public MemoryUpdate(
+ @JsonProperty("path") String path,
+ @JsonProperty("value") Object value,
+ @JsonProperty("objectCreation") boolean objectCreation) {
Review Comment:
Could we reject objectCreation=true with a non-null value in this
constructor? MemoryUpdateReplayer ignores the value for object creation, so
accepting this invalid combination can silently hide malformed updates.
##########
runtime/src/test/java/org/apache/flink/agents/runtime/actionstate/ActionStateSerdeTest.java:
##########
@@ -61,11 +61,13 @@ public void testActionStateSerializationDeserialization()
throws Exception {
MemoryUpdate sensoryMemoryUpdate = new MemoryUpdate("sm.test.path",
"sm test value");
MemoryUpdate shortTermMemoryUpdate = new MemoryUpdate("stm.test.path",
"stm test value");
+ MemoryUpdate objectCreationUpdate = new MemoryUpdate("stm.test.obj",
null, true);
Review Comment:
Could we add a legacy JSON case with objectCreation omitted and assert that
it defaults to false? The current round trip only covers records written by the
new serializer.
--
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]
