Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1764
---
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 enab
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-215399616
Perfect!
Thanks a lot @rmetzger
---
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 n
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-215399436
One minor remark for the future (I'll fix it when merging): Please include
the JIRA ID into the commit message.
I'm going to merge this change once travis gives
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/1764#discussion_r61411342
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/AscendingTimestampExtractor.java
---
@@ -20,123 +20,18 @@
i
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-215374376
Thanks a lot @rmetzger
---
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 f
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-215374186
Hi
I'm sorry. I'll take a look at it immediately
---
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 kl0u commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-215373597
Could somebody review 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 t
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-210462831
If everything is ok, could somebody merge this?
It has been some time that there is no activity here.
---
If your project is set up for it, you can reply to this email
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-208247654
Could somebody review 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 th
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-205294750
Hello @StephanEwen ,
I integrated your comments.
Please review and let me know if there is any other issue.
---
If your project is set up for it, you can reply to
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-205280465
We still have the issue of the name. I don't like to have "lateness" in the
name, as "lateness" so far refers to either events coming behind watermarks
(nothing to d
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-205233984
Hello @StephanEwen
I totally agree with the comments.
I rearranged the extractors and integrated your comments on the
documentation.
Please review.
---
If you
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-204508742
The code looks good now.
- How about we move all the convenience timestamp extractors / watermark
generators into a separate packe `.functions.timestamps`?
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-199299432
Thanks a lot @rmetzger and @mxm for the comments. I integrated them.
Please review and let me know if there is anything missing.
---
If your project is set up for it, y
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-199219824
Once the false comments have been fixed, the change is good to merge IMO.
---
If your project is set up for it, you can reply to this email and have your
reply appear o
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/1764#discussion_r56802763
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/FixedAllowedLatenessWatermarkEmitter.java
---
@@ -0,0 +1,84 @@
+/*
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/1764#discussion_r56802608
--- Diff: docs/apis/streaming/event_timestamps_watermarks.md ---
@@ -308,6 +269,72 @@ class TimeLagWatermarkGenerator extends
AssignerWithPeriodicWatermarks[
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-197821005
Thanks a lot @mxm @rmetzger and @StephanEwen . I agree with @mxm , that is
why I added references to the new timestampExtractor in the javadocs of the
assignTimestampsAndWa
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-197821121
Please 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
ena
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-197937643
Thanks @kl0u. In addition to the JavaDoc, we should also document this in
the streaming Programming Guide. Here:
https://ci.apache.org/projects/flink/flink-docs-master/apis/
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-197248226
Should this be referenced in the docs and perhaps also contain the example
implementation of the LongExtractor? My gut feeling says that this might be
dead code otherwise.
Github user kl0u commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-196422054
@StephanEwen @rmetzger I have integrated your comments.
Please review!
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/1764#discussion_r56008125
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/TrailingWatermarkTimestampExtractor.java
---
@@ -0,0 +1,71 @@
+/*
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1764#issuecomment-195489648
I think this is a useful tool. Few comments, though:
- You can still have a negative underflow (wrap around, positive) in the
watermark generation.
-
Github user kl0u commented on a diff in the pull request:
https://github.com/apache/flink/pull/1764#discussion_r55020209
--- Diff:
flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/DataStream.scala
---
@@ -750,6 +750,34 @@ class DataStream[T](stream: JavaSt
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/1764#discussion_r55019932
--- Diff:
flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/DataStream.scala
---
@@ -750,6 +750,34 @@ class DataStream[T](stream: Ja
GitHub user kl0u opened a pull request:
https://github.com/apache/flink/pull/1764
FLINK-3428: Adds a fixed time trailing watermark timestamp extractor.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kl0u/flink flink-3428
Altern
27 matches
Mail list logo
