[GitHub] flink issue #3479: [FLINK-5929] Allow Access to Per-Window State in ProcessW...

[aljoscha]

Github user aljoscha commented on the issue:

https://github.com/apache/flink/pull/3479
  
Hehe, thanks! 😄 


---
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 #3479: [FLINK-5929] Allow Access to Per-Window State in ProcessW...

[sjwiesman]

Github user sjwiesman commented on the issue:

https://github.com/apache/flink/pull/3479
  
Done! Thank you for for helping me get this feature merged in. This has to 
be one of the most painless commits I've ever made to an open source project of 
this size. 


---
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 #3479: [FLINK-5929] Allow Access to Per-Window State in ProcessW...

[aljoscha]

Github user aljoscha commented on the issue:

https://github.com/apache/flink/pull/3479
  
Thanks for implementing this! 😃 

I just merged, could you please close this PR?


---
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 #3479: [FLINK-5929] Allow Access to Per-Window State in ProcessW...

[aljoscha]

Github user aljoscha commented on the issue:

https://github.com/apache/flink/pull/3479
  
Thanks!


---
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 #3479: [FLINK-5929] Allow Access to Per-Window State in ProcessW...

[sjwiesman]

Github user sjwiesman commented on the issue:

https://github.com/apache/flink/pull/3479
  
Pushed the fix, I had to update SideOutputsITCase so the 
ProcessAllWindowFunctions had a noop clear method. All tests passed locally, 
take a look


---
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 #3479: [FLINK-5929] Allow Access to Per-Window State in ProcessW...

[aljoscha]

Github user aljoscha commented on the issue:

https://github.com/apache/flink/pull/3479
  
Ok, please ping me when you pushed the fix. 😃 


---
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 #3479: [FLINK-5929] Allow Access to Per-Window State in ProcessW...

[sjwiesman]

Github user sjwiesman commented on the issue:

https://github.com/apache/flink/pull/3479
  
It looks like when I rebased on master I broke one of the scala side 
outputs test. I'm going to push a fix right now but it won't change any of the 
code surrounding this pr. 


---
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 #3479: [FLINK-5929] Allow Access to Per-Window State in ProcessW...

[aljoscha]

Github user aljoscha commented on the issue:

https://github.com/apache/flink/pull/3479
  
Don't worry. 😃 Is it ready for another review pass now?


---
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 #3479: [FLINK-5929] Allow Access to Per-Window State in ProcessW...

[sjwiesman]

Github user sjwiesman commented on the issue:

https://github.com/apache/flink/pull/3479
  
sorry for the delay, things got crazy at work. let me know if there are any 
issues. 


---
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 #3479: [FLINK-5929] Allow Access to Per-Window State in ProcessW...

[sjwiesman]

Github user sjwiesman commented on the issue:

https://github.com/apache/flink/pull/3479
  
@aljoscha I made the changes you asked for. Just a heads up, there are a 
number of files that were superficially changed when migrating from apply -> 
process but are otherwise untouched. 


---
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 #3479: [FLINK-5929] Allow Access to Per-Window State in ProcessW...

[aljoscha]

Github user aljoscha commented on the issue:

https://github.com/apache/flink/pull/3479
  
One quick initial remark: instead of each time having an anonymous inner 
class for the `Context` you can create a reusable class for that like this:
```
class InternalProcessWindowContext
extends ProcessWindowFunction.Context {

W window;
InternalWindowFunction.InternalWindowContext internalContext;

InternalProcessWindowContext(ProcessWindowFunction 
function) {
function.super();
}

@Override
public W window() {
return window;
}

@Override
public KeyedStateStore windowState() {
return internalContext.windowState();
}

@Override
public KeyedStateStore globalState() {
return internalContext.globalState();
}
}
```

The `function.super()` call in there makes it work even though `Context` is 
itself defined as an inner abstract class of `ProcessWindowFunction`. It's a 
bit of black magic and not really too well known, I think. 😉


---
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 #3479: [FLINK-5929] Allow Access to Per-Window State in ProcessW...

[aljoscha]

Github user aljoscha commented on the issue:

https://github.com/apache/flink/pull/3479
  
Thanks @sjwiesman! I'll have a look.


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