shunping commented on code in PR #30317:
URL: https://github.com/apache/beam/pull/30317#discussion_r1492753170
##########
model/fn-execution/src/main/proto/org/apache/beam/model/fn_execution/v1/beam_fn_api.proto:
##########
@@ -1075,6 +1111,42 @@ message StateClearRequest {}
// A response to clear state.
message StateClearResponse {}
Review Comment:
There is a discussion for this in the original design doc.
https://docs.google.com/document/d/1U77sAvE6Iy9XsVruRYHxPdFji7nqS6HPi1XU8fhyrxs/edit?disco=AAAAH0MsNik
I think @kennknowles mentioned he preferred separate messages.
Personally, I prefer the way you propose to re-use the Get/Append/Clear
messages by putting optional fields there for ordered list state.
However, I hesitate to do that because (1) it will introduce new optional
fields to an existing proto message and (2) regarding extensibility and
flexibility, it may be easier to have separate messages for ordered list and
any future states that doesn't fit in current Get/Append/Clear paradigm?
WDYT?
--
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]