kennknowles commented on code in PR #30317:
URL: https://github.com/apache/beam/pull/30317#discussion_r1500839373
##########
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:
I still agree with my comment there: "it looks like a 'do everything'
request/response protocol and the key holds all the information." That comment
may seem to be about style, but it is really about transparency, debuggability,
readability.
I view whatever "consistency" we have amongst the state requests as
coincidence at best and a mistake at worst. _The_ reason that different kinds
of state exist is because they support different methods. There is a reason
that the only method they have in common in Java (where it originated) is
`clear()`. Apparently the design decision was to model `methodA, methodB,
methodC` as `oneBigMethod("actually do A"), oneBigMethod("actually do B"),
oneBigMethod("actually do C")`. I just think that is not as good as being
straightforward.
--
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]
