[GitHub] flink issue #3503: [FLINK-5995][checkpoints] fix Get a Exception when creati...

[StephanEwen]

Github user StephanEwen commented on the issue:

https://github.com/apache/flink/pull/3503
  
Looks good, merging this...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3503: [FLINK-5995][checkpoints] fix Get a Exception when creati...

[sunjincheng121]

Github user sunjincheng121 commented on the issue:

https://github.com/apache/flink/pull/3503
  
HI,guys, Welcome anyone to review this PR.
@StephanEwen Can you continue review this PR.
Best，
SunJIncheng


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3503: [FLINK-5995][checkpoints] fix Get a Exception when creati...

[sunjincheng121]

Github user sunjincheng121 commented on the issue:

https://github.com/apache/flink/pull/3503
  
@StephanEwen I added a test to make sure the `ExecutionConfig` was passed, 
but I'm not sure whether I understand you correctly. feel free to tell me If it 
is not right.
Thanks,
SunJincheng 



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3503: [FLINK-5995][checkpoints] fix Get a Exception when creati...

[StephanEwen]

Github user StephanEwen commented on the issue:

https://github.com/apache/flink/pull/3503
  
Looks good. I would actually like to extend the test by one more, just to 
make sure that the proper `ExecutionConfig` is passed (which is not currently 
tested - the execution config in the test is null).

A good way to do that would be to create a state of some non-standard type 
(like `File`) which goes to Kryo and then check that Kryo has the proper 
registration from the execution config.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3503: [FLINK-5995][checkpoints] fix Get a Exception when creati...

[sunjincheng121]

Github user sunjincheng121 commented on the issue:

https://github.com/apache/flink/pull/3503
  
@StephanEwen I had update the PR. I appreciated if you can review this PR.
Best,
SunJincheng


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3503: [FLINK-5995][checkpoints] fix Get a Exception when creati...

[StephanEwen]

Github user StephanEwen commented on the issue:

https://github.com/apache/flink/pull/3503
  
That sounds reasonable!

On Mar 10, 2017 4:07 PM, "Jincheng Sun"  wrote:

> Hi,@StephanEwen , Thanks a lot for your
> comment. I seriously think about where to add the test more is reasonable,
> because the change of this PR is OperatorStateBackend, so I added the
> test case injava/ src/test/java/org/apache/flink/runtime/state/
> operatorStateBackendTest.java Is this make sense for you? feel free to
> tell me if it is incorrect.
> Best,
> SunJincheng
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> , or 
mute
> the thread
> 

> .
>



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3503: [FLINK-5995][checkpoints] fix Get a Exception when creati...

[sunjincheng121]

Github user sunjincheng121 commented on the issue:

https://github.com/apache/flink/pull/3503
  
Hi,@StephanEwen, Thanks a lot for your comment. I seriously think about 
where to add the test more is reasonable, because the change of this PR is 
`OperatorStateBackend`, so I added the test case in`java/ 
src/test/java/org/apache/flink/runtime/state/operatorStateBackendTest.java` Is 
this make sense for you? feel free to tell me if it is incorrect.
Best,
SunJincheng


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3503: [FLINK-5995][checkpoints] fix Get a Exception when creati...

[StephanEwen]

Github user StephanEwen commented on the issue:

https://github.com/apache/flink/pull/3503
  
Thanks for finding and fixing this, important fix!

It would be great if you could add a test for this, to make sure the bug is 
not accidentally re-introduced later by someone. You can follow the pattern of 
this test: 
`flink-streaming-java/src/test/java/org/apache/flink/streaming/api/operators/StateDescriptorPassingTest.java`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---