You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/08/31 09:08:05 UTC

[GitHub] [flink-statefun] tzulitai opened a new pull request #137: [FLINK-19102] [core, sdk] Make StateBinder a per-FunctionType entity

tzulitai opened a new pull request #137:
URL: https://github.com/apache/flink-statefun/pull/137


   This PR is based on top of #136 because it touches a few overlapping classes.
   Only last 2 commits are relevant.
   
   ---
   
   Prior to this PR, a single `StateBinder` instance handles binding state objects to Flink state for multiple `FunctionType`s.
   
   With these changes, this is changed so that each `FunctionType` has its own `StateBinder` (specifically, a `FlinkStateBinder`).
   This allows a cleaner separation of concerns in the `PersistedStateRegistry`, so that the information about which function type the registry is bounded is not leaked into the SDK classes, but instead encapsulated within the `StateBinder` being used at runtime.
   
   ---
   
   ## Verifying this change
   
   The existing tests in `PersistedStatesTest` cover the changes introduced by this refactoring.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-statefun] igalshilman commented on a change in pull request #137: [FLINK-19102] [core, sdk] Make StateBinder a per-FunctionType entity

Posted by GitBox <gi...@apache.org>.
igalshilman commented on a change in pull request #137:
URL: https://github.com/apache/flink-statefun/pull/137#discussion_r480276203



##########
File path: statefun-sdk/src/main/java/org/apache/flink/statefun/sdk/state/PersistedAppendingBuffer.java
##########
@@ -160,6 +160,13 @@ public void clear() {
     accessor.clear();
   }
 
+  @Override

Review comment:
       I think that it would be somewhat not intuitive for users not to see the actual value, what do you think?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-statefun] igalshilman commented on a change in pull request #137: [FLINK-19102] [core, sdk] Make StateBinder a per-FunctionType entity

Posted by GitBox <gi...@apache.org>.
igalshilman commented on a change in pull request #137:
URL: https://github.com/apache/flink-statefun/pull/137#discussion_r480276203



##########
File path: statefun-sdk/src/main/java/org/apache/flink/statefun/sdk/state/PersistedAppendingBuffer.java
##########
@@ -160,6 +160,13 @@ public void clear() {
     accessor.clear();
   }
 
+  @Override

Review comment:
       I think that it would be somewhat not intuitive for users not to see the actual value, what do you think?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [flink-statefun] tzulitai closed pull request #137: [FLINK-19102] [core, sdk] Make StateBinder a per-FunctionType entity

Posted by GitBox <gi...@apache.org>.
tzulitai closed pull request #137:
URL: https://github.com/apache/flink-statefun/pull/137


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org