Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1704
---
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
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-205288351
It's been a pleasure, thanks for the patience and for guiding me through
the iterations. :smiley:
---
If your project is set up for it, you can reply to this
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-205234947
@StephanEwen Thanks for the feedback, I've addressed the points you
highlighted and took the time to add some missing Scaladoc to the `DataStream`
extension.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-204500365
Looks pretty good. I noticed three remaining things:
- Sometimes, the package and the directory trees are different (example:
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r58244518
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/impl/acceptPartialFunctions/OnDataSet.scala
---
@@ -0,0 +1,111 @@
+/*
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-204334976
Note on the tests of the streaming extensions: I couldn't find a more
specific class than `SingleOutputStreamOperator[_]`, thus assertion may not be
very
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-202778390
Sounds great @stefanobaghino. I think you can push your work to this PR as
well since it is all related to the partial function support. Looking forward
having
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-201325574
@tillrohrmann I moved forward on the batch extensions: I added support for
anonymous partial functions on `where` and `equalsTo` for joins and co-group
Github user stefanobaghino commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r57452214
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/package.scala
---
@@ -0,0 +1,201 @@
+/*
+ * Licensed to the Apache
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-199840360
Your 2. proposal looks more natural to me because what you receive as input
to a user function is in fact a stream of data. So if this solution is
feasible, then I
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-199837531
A quick status update: unfortunately I have had no time to work on this in
the past week, I plan to get back to the fixes on Thursday or Friday.
In the
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-196325179
Yes I think they will be removed at runtime. Thus, I guess it should also
be fine to test for `isInstanceOf[StreamMap[_, _]]`. If you also want to check
the
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-196321443
@tillrohrmann I just have one minor concern regarding the tests: would
`isInstanceOf[StreamMap[Int, Int]]` work? Wouldn't the generic type parameters
(the two
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-196320836
@tillrohrmann Thanks! Outstanding review, it's great to have some guidance
when approaching a new project; thank you for the tips on testing as well, it
turned
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r56001580
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/package.scala
---
@@ -0,0 +1,201 @@
+/*
+ * Licensed to the Apache
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-196314763
@stefanobaghino, really good work :-) I think we're close to get this
merged. I had some minor comments.
Concerning the testing, I agree with @StephanEwen
Github user stefanobaghino commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r56000181
--- Diff:
flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/extensions/acceptPartialFunctions/OnWindowedStream.scala
---
@@
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55999131
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/package.scala
---
@@ -0,0 +1,201 @@
+/*
+ * Licensed to the Apache
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55998933
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/acceptPartialFunctions/OnDataSet.scala
---
@@ -0,0 +1,104 @@
+/*
+ *
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55997348
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/acceptPartialFunctions/OnDataSet.scala
---
@@ -0,0 +1,104 @@
+/*
+ *
Github user stefanobaghino commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55996335
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/acceptPartialFunctions/OnDataSet.scala
---
@@ -0,0 +1,104 @@
+/*
+
Github user stefanobaghino commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55995777
--- Diff: docs/apis/scala_api_extensions.md ---
@@ -0,0 +1,392 @@
+---
+title: "Scala API Extensions"
+# Top-level navigation
Github user stefanobaghino commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55995517
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/package.scala
---
@@ -0,0 +1,201 @@
+/*
+ * Licensed to the Apache
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55995289
--- Diff:
flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/extensions/package.scala
---
@@ -0,0 +1,202 @@
+/*
+ *
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55995217
--- Diff:
flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/extensions/acceptPartialFunctions/OnWindowedStream.scala
---
@@
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55994602
--- Diff:
flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/extensions/acceptPartialFunctions/OnWindowedStream.scala
---
@@
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55994370
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/package.scala
---
@@ -0,0 +1,201 @@
+/*
+ * Licensed to the Apache
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55994329
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/package.scala
---
@@ -0,0 +1,201 @@
+/*
+ * Licensed to the Apache
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55994195
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/package.scala
---
@@ -0,0 +1,201 @@
+/*
+ * Licensed to the Apache
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55993999
--- Diff: docs/apis/scala_api_extensions.md ---
@@ -0,0 +1,392 @@
+---
+title: "Scala API Extensions"
+# Top-level navigation
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55993670
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/acceptPartialFunctions/OnGroupedDataSet.scala
---
@@ -0,0 +1,75 @@
+/*
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55993657
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/acceptPartialFunctions/OnGroupedDataSet.scala
---
@@ -0,0 +1,75 @@
+/*
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55993580
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/acceptPartialFunctions/OnDataSet.scala
---
@@ -0,0 +1,104 @@
+/*
+ *
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r55993625
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/acceptPartialFunctions/OnDataSet.scala
---
@@ -0,0 +1,104 @@
+/*
+ *
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-195818369
Thanks for the feedback. Basically the tests so far have been a carbon copy
of the ones run on the operators I delegate to from the extensions. I agree
that this
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-195491606
This pull request adds a lot of tests (which is actually good), but all
tests fire up a cluster to execute many programs. But need to somehow get this
down, as
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-195290267
@tillrohrmann The tests failure seem to be flaky, I've re-run them all on
our fork and they're all green now (after a couple of retries).
---
If your project is
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-192259307
@tillrohrmann Yes, I've already fixed them locally, thanks for the notice
and for pinging me. I'm working on the documentation, sorry if it's taking me a
little
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-191799798
There are scalastyle violations in the code:
```
error
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-188790808
@StephanEwen I had to restore some of the context bounds on `ClassTag` to
make it compile, apparently the delegated methods use them; I've rebased with
the
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-188409068
This should cover the missing implementations. I also included the tests I
used to test the functionality, let me know if you prefer a wider coverage.
I'll
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-188352497
@tillrohrmann I got mixed up reading your proposal and just got what you
meant with the `extensions.acceptPartialFunctions` implicit conversion, thanks
for the
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-188343795
:+1: @StephanEwen I started working on the extension before the removal,
will fix this as well, thanks for the feedback.
---
If your project is set up for it,
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-188335368
Looks very nice in my opinion.
Could you remove the `ClassTag` context bounds? We recently removed them
from `DataStream`, because they are not needed.
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-188329671
Of course, you're right. Also in the batch guide :-)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-188325067
Just the streaming guide? Not on both the streaming and batch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-188320230
How do you want to do the implicit conversion if you return a `DataSet[T]`.
If it makes the code more readable, then I think it's a good idea :-)
We should
Github user stefanobaghino commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-188301093
Thanks @tillrohrmann, I'll fix the errors in the comments and add the
missing methods and extensions.
Regarding the import mode, I agree with you. I
Github user stefanobaghino commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r53950863
--- Diff:
flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/extensions/acceptPartialFunctions/package.scala
---
@@ -0,0
Github user stefanobaghino commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r53950747
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/acceptPartialFunctions/package.scala
---
@@ -0,0 +1,174 @@
+/*
+ *
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-188289230
Thanks for your contribution @stefanobaghino. I really like this feature a
lot :-)
Currently, the implementation is not complete, because the supported set
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1704#issuecomment-188289888
It would also be great to add some tests to make sure that the import is
working. Furthermore, it would be great to add documentation for the extension
feature.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r53948314
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/acceptPartialFunctions/package.scala
---
@@ -0,0 +1,174 @@
+/*
+ *
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r53948074
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/acceptPartialFunctions/package.scala
---
@@ -0,0 +1,174 @@
+/*
+ *
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r53948137
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/acceptPartialFunctions/package.scala
---
@@ -0,0 +1,174 @@
+/*
+ *
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r53946205
--- Diff:
flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/extensions/acceptPartialFunctions/package.scala
---
@@ -0,0 +1,133
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r53947242
--- Diff:
flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/extensions/acceptPartialFunctions/package.scala
---
@@ -0,0 +1,133
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r53947144
--- Diff:
flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/extensions/acceptPartialFunctions/package.scala
---
@@ -0,0 +1,133
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r53946101
--- Diff:
flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/extensions/acceptPartialFunctions/package.scala
---
@@ -0,0 +1,133
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r53946030
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/acceptPartialFunctions/package.scala
---
@@ -0,0 +1,174 @@
+/*
+ *
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1704#discussion_r53945929
--- Diff:
flink-scala/src/main/scala/org/apache/flink/api/scala/extensions/acceptPartialFunctions/package.scala
---
@@ -0,0 +1,174 @@
+/*
+ *
GitHub user stefanobaghino opened a pull request:
https://github.com/apache/flink/pull/1704
[FLINK-1159] Case style anonymous functions not supported by Scala API
The proposed API extension methods would allow developers to pass a pattern
matching anonymous function so that they
62 matches
Mail list logo
