Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2176#discussion_r69148656
--- Diff: flink-contrib/docker-flink/README.md ---
@@ -1,80 +1,75 @@
-#Apache Flink cluster deployment on Docker using Docker-Compose
+Apache Flink
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2176
Re 1. I think this should be fine. There is some dynamic code generation
but this uses Janino as a library so that shouldn't be a problem.
Re 2. I'm not aware of any but it shou
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2176
One last thing I would like to try is running a job from an existing Flink
installation using `$FLINK_HOME/bin/flink run -m `
as described in the README.
I suspect it has something to do
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2154#discussion_r69262604
--- Diff: docs/apis/streaming/windows.md ---
@@ -24,1023 +24,608 @@ specific language governing permissions and
limitations
under the License
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2154#discussion_r69263156
--- Diff: docs/apis/streaming/windows.md ---
@@ -24,1023 +24,608 @@ specific language governing permissions and
limitations
under the License
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2154#discussion_r69263379
--- Diff: docs/apis/streaming/windows.md ---
@@ -24,1023 +24,608 @@ specific language governing permissions and
limitations
under the License
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2154#discussion_r69263452
--- Diff: docs/apis/streaming/windows.md ---
@@ -24,1023 +24,608 @@ specific language governing permissions and
limitations
under the License
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2154#discussion_r69264040
--- Diff: docs/apis/streaming/windows.md ---
@@ -24,1023 +24,608 @@ specific language governing permissions and
limitations
under the License
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2154#discussion_r69264177
--- Diff: docs/apis/streaming/windows.md ---
@@ -24,1023 +24,608 @@ specific language governing permissions and
limitations
under the License
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2154#discussion_r69264437
--- Diff: docs/apis/streaming/windows.md ---
@@ -24,1023 +24,608 @@ specific language governing permissions and
limitations
under the License
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2154#discussion_r69264695
--- Diff: docs/apis/streaming/windows.md ---
@@ -24,1023 +24,608 @@ specific language governing permissions and
limitations
under the License
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2154
Thanks for the thorough review, @uce. I incorporated the comments.
I left the section about triggers and lateness a bit vague on purpose since
we will probably change some stuff there after
Github user aljoscha closed the pull request at:
https://github.com/apache/flink/pull/2154
---
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 aljoscha commented on the issue:
https://github.com/apache/flink/pull/2176
Yes, this is exactly what I was trying on OS X. I'm quickly setting up a
ubuntu VM to see if it works there.
---
If your project is set up for it, you can reply to this email and have your
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2176
You were right, I did exactly the same thing I did on OS X on a new Ubuntu
16.04 installation and it worked. ð
---
If your project is set up for it, you can reply to this email and have your
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2176#discussion_r69304467
--- Diff: flink-contrib/docker-flink/README.md ---
@@ -1,80 +1,75 @@
-#Apache Flink cluster deployment on Docker using Docker-Compose
+Apache Flink
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2176
I had two more comments about the README but after that it should be good
to merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2176#discussion_r69304962
--- Diff: flink-contrib/docker-flink/README.md ---
@@ -1,80 +1,75 @@
-#Apache Flink cluster deployment on Docker using Docker-Compose
+Apache Flink
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2176#discussion_r69307034
--- Diff: flink-contrib/docker-flink/README.md ---
@@ -1,80 +1,75 @@
-#Apache Flink cluster deployment on Docker using Docker-Compose
+Apache Flink
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2176#discussion_r69307221
--- Diff: flink-contrib/docker-flink/README.md ---
@@ -1,80 +1,75 @@
-#Apache Flink cluster deployment on Docker using Docker-Compose
+Apache Flink
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2176
That's great to hear! I'll write something on the Beam ML 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 yo
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2176
I merged it, thanks again for your work!
---
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
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2196
Thanks for the fix. I'll rename the commit to something like `[FLINK-4134]
Retire Late Windows/Elements in WindowOperator` because it is a) shorter and b)
clearly describes what the change
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2196
Could you please close this PR if it is not automatically closed. Thanks
again for the fix. ð
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2157
Thanks for your work again! I merged it. ð
---
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
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2199#discussion_r69552327
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/util/WindowingTestHarness.java
---
@@ -0,0 +1,217 @@
+/**
+ * Licensed to
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2199#discussion_r69552396
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/util/WindowingTestHarness.java
---
@@ -0,0 +1,217 @@
+/**
+ * Licensed to
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2199#discussion_r69552555
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/util/WindowingTestHarness.java
---
@@ -0,0 +1,217 @@
+/**
+ * Licensed to
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2199
Looks good! I just had some minor comments. After fixing those it should be
good to merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
GitHub user aljoscha opened a pull request:
https://github.com/apache/flink/pull/2202
[FLINK-4149] Fix Serialization of NFA in AbstractKeyedCEPPatternOperator
NFA is Serializable and has readObject()/writeObject() methods. In
AbstractKeyedCEPPatternOperator a KryoSerializer was
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2202
R: @tillrohrmann for review
---
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
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2177
Sorry for chiming in late, but can't - `heap-cutoff-ratio` and
`heap-cutoff-min` be general parameters that apply to all VMs we start?
Same for `master.env.*` and `taskmanage
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2177
My thinking was that Mesos maybe didn't require this, yes. Having it for
Yarn only makes for a better user experience right now. So we could leave it
under the "container" namespace
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2177
Yeah, then let's leave them as they were since we don't know yet what mesos
will require there.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2199
Thanks for your work. ð I merged it. Could you please take care of
closing this PR and the issue?
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2202
Thanks for the review @tillrohrmann! The `commons.io` was just an IDE
mess-up. The `equals` I will fix.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2240
@iemejia This is now good to merge?
From my side it looks good!
---
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
Github user aljoscha closed the pull request at:
https://github.com/apache/flink/pull/2202
---
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 aljoscha commented on the issue:
https://github.com/apache/flink/pull/2231
I agree with @tzulitai that it would be nice if you could have minimum code
duplication in the long-run but it might not be possible with the current
design of the consumers.
What about
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2240
Would the Flink image be tied to a specific Flink version?
---
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
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2240
I merged it. Could you please close this PR if it doesn't close
automatically.
Thanks for the work! ð
---
If your project is set up for it, you can reply to this email and have
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2240
Yes, that would probably be good.
---
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
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2240
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
GitHub user aljoscha opened a pull request:
https://github.com/apache/flink/pull/2266
[FLINK-4229] Do not start any Metrics Reporter by default
@rmetzger Is this what you had in mind when you mentioned that the
JMXReporter should not be active by default because it starts a lot of
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2266
@zentol I thought about that as well but wanted to keep the change to one
issue. ð
I'll change it now.
---
If your project is set up for it, you can reply to this email and have
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2265
Yes, @wuchong's suggestion for the Java Table API seems more extensible.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2265
Ah ok, thats perfect! (about infix and postfix)
---
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
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2263
Nice pice of code! I finally understood how it works... ð
Some remarks about the code: in some places there are method names that
seem to stem from an initial implementation but don&#
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2263
Oh, and I forgot: the checking for the correct number of elements can be
moved out of the window function and into the test itself, like this:
```
JobExecutionResult result
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2266
@zentol I addressed the stuff. Do you think this is alright 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
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2266
Dammit, I thought about the doc but then forgot... changing.
---
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
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2266#discussion_r71518447
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/jobmanager/JobManagerMetricTest.java
---
@@ -50,9 +50,10 @@
public void
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2266
Thanks for reviewing! I'll merge in a bit.
---
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 fe
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2270
Perfect! I'll merge right away.
---
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
en
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2270
Merged it. Could you please close this PR and mark the Jira as resolved?
---
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
Github user aljoscha closed the pull request at:
https://github.com/apache/flink/pull/2266
---
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 aljoscha opened a pull request:
https://github.com/apache/flink/pull/2273
[FLINK-4238] Only allow/require query for Tuple Stream in CassandraSink
R: @zentol for review
You can merge this pull request into a Git repository by running:
$ git pull https://github.com
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2263
Thanks for your work! I merged it. Could you please close this PR if it
isn't closed automatically.
---
If your project is set up for it, you can reply to this email and have your
reply appe
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2273
Thanks for review! I moved the checks to the specific builders.
---
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
GitHub user aljoscha opened a pull request:
https://github.com/apache/flink/pull/2278
[FLINK-4239] Set Default Allowed Lateness to Zero and Make Triggers Nâ¦
R: @kl0u for review
You can merge this pull request into a Git repository by running:
$ git pull https://github.com
Github user aljoscha closed the pull request at:
https://github.com/apache/flink/pull/2273
---
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 aljoscha commented on the pull request:
https://github.com/apache/flink/commit/19dae21b00cfbf68ee64af80672d25974a3cd346#commitcomment-18339338
That seems like a good idea, yes. Still not have the JMXReporter active by
default or now have it active by default again
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2269
Very good work!
I know we discussed before whether to check for inactivity in a different
thread or in `invoke()`. There's actually a third option that I'm showcasing in
th
GitHub user aljoscha opened a pull request:
https://github.com/apache/flink/pull/2279
[FLINK-4229] Only start JMX server when port is specified
The JMX reporter now only starts an extra server if a port (or port range)
was specified. Otherwise, it will just have the default local
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2279
@StephanEwen did you merge 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
GitHub user aljoscha opened a pull request:
https://github.com/apache/flink/pull/2285
[FLINK-4246] Allow Specifying Multiple Metrics Reporters
This also updates documentation and tests.
Reporters can now be specified like this:
metrics.reporters: foo,bar
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2285#discussion_r71896724
--- Diff:
flink-core/src/main/java/org/apache/flink/metrics/MetricRegistry.java ---
@@ -75,77 +78,92 @@ public MetricRegistry(Configuration config
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2285
Thanks for the thorough review @zentol! I'm addressing the comments in a
new commit.
About the move to `MetricConfig`, I think the Configuration could be
extended for that case to mov
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2285
Jip, but that's not done very often, only when instantiating the
`MetricRegistry`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as wel
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2269
Not really, the reason for having an ITCase is just that they really
exercise the sink embedded in a proper Flink job, which might bring up
interactions that where overlooked when writing a test
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2287
I'm checking now but I'm assuming it will fix it from looking at the code.
Could we also a a test for this?
---
If your project is set up for it, you can reply to this email and have
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2269
This is caused by `setInputType()` not being called on
`SequenceFileWriter`. In the test, you can call `setInputType` on the
`BucketingSink` once with the input `TypeInformation` and a `new
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2287
This does in fact fix the problem. Running smoothly 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
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2287
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
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2287
Test looks good!
Please go ahead and merge if you're ready.
---
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 pr
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2277
This doesn't work for some cases. Consider, when you change
`testCleanupTimerWithEmptyListStateForTumblingWindows()` to this:
```
@Test
public
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2277#discussion_r72067210
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/operators/windowing/WindowOperator.java
---
@@ -408,11 +410,22 @@ public
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2278
Thanks for the review! The change you proposed really makes the code a lot
nicer. ð
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2269
Thanks for moving the tests! It should be ok to leave these other two
ITCases.
I'll merge the commits into one once I can find some time to look at the
tests and merge it.
---
If
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2226
Why did you remove the `@PublicEvolving` annotations? I think technically
they are not required since the code is not in the core or runtime package
anymore but it's still a good hint for
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2278#discussion_r72076841
--- Diff:
flink-tests/src/test/java/org/apache/flink/test/windowing/sessionwindows/SessionWindowITCase.java
---
@@ -114,8 +115,8 @@ private void runTest
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2278#discussion_r72076897
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/operators/windowing/WindowOperatorTest.java
---
@@ -1409,7 +1409,7
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2278
I addressed the other comments as well.
---
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
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2226
That sounds like a good solution, yes!
---
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
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2285
I rebased this on top of #2226
---
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
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2285#discussion_r72091326
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/MetricRegistry.java
---
@@ -74,81 +76,97 @@ public MetricRegistry(Configuration
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2285
There are tests in `JMXReporterTest` that instantiate multiple reporters
but I'll also add a simple on to `MetricRegistryTest`.
---
If your project is set up for it, you can reply to this
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2285
I addressed the comments.
---
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
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2277#discussion_r72092981
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/state/AppendingState.java
---
@@ -46,8 +46,14 @@
* operator instance. If state
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2277#discussion_r72093243
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/operators/windowing/MergingWindowSet.java
---
@@ -80,8 +80,10 @@ public
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2277
Looks good now! I had some minor inline comments.
---
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
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2285
Ahh, you're right, I'll fix that.
---
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 featu
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2285
I added a new test, is that what you had in mind?
---
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
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2285
Ok, now we first have to merge the other 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
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2226
How confident are we? ð Should we wait for travis to finish or can we
merge now?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2226
Ok, I think anything else we'll discover in the release testing. If any ...
LGTM!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitH
Github user aljoscha closed the pull request at:
https://github.com/apache/flink/pull/2285
---
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 aljoscha commented on the issue:
https://github.com/apache/flink/pull/2277
Could you please close? And thanks for your work! ð
---
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
Github user aljoscha commented on the issue:
https://github.com/apache/flink/pull/2305
@StephanEwen I think technically it doesn't break the API because
`SingleOutputStreamOperator` is a subclass of `DataStream`, right? (It might
break binary compatibility though, becaus
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/2282#discussion_r73349912
--- Diff: docs/apis/table.md ---
@@ -606,6 +606,28 @@ Table result = in.orderBy("a.asc");
+
+
301 - 400 of 5077 matches
Mail list logo
