Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5283
Merging ..
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5188
Thanks! Merging this ...
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5283
Thanks, good catch!
@elbaulp can you include the `[hotfix]` and `[doc]` tags to the commit
message? All our commit requires proper tags to indicate the issue id (or
`hotfix` for typos
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5188
Thank you for the review @pnowojski! Have addressed your comments.
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/3915
Hi @zjureel, I went ahead to address my own review / concerns with the
change in another PR that is based on your current work: #5282. I hope that is
okay, and would be great if you would like
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5282
cc @zjureel
---
GitHub user tzulitai opened a pull request:
https://github.com/apache/flink/pull/5282
[FLINK-6352] [kafka] Timestamp-based offset configuration for
FlinkKafkaConsumer
## What is the purpose of the change
This PR is based on @zjureel's initial efforts on the feature
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5214
Thanks for the PR @tony810430. The changes LGTM, will merge this a bit
later.
---
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5182#discussion_r160841457
--- Diff:
flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/internals/KinesisDataFetcher.java
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5182#discussion_r160841254
--- Diff:
flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/internals/KinesisDataFetcher.java
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5200
As discussed offline:
Will re-open a PR for this that avoids introducing a new interface. I agree
that it would make more sense to introduce new interface when we actually
decide to go
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/3915#discussion_r160677582
--- Diff:
flink-connectors/flink-connector-kafka-0.10/src/main/java/org/apache/flink/streaming/connectors/kafka/internal/KafkaConsumerCallBridge010.java
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/3915#discussion_r160676392
--- Diff:
flink-connectors/flink-connector-kafka-0.10/src/main/java/org/apache/flink/streaming/connectors/kafka/internal/KafkaConsumerCallBridge010.java
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5124
Will merge this by the end of the week, given that there are no other
objections ...
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5243
I can't recall any license issues with shading the ES connectors ... will
keep an extra eye and double check on that before merging this.
@NicoK thanks a lot for your work on this. I'll
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5243#discussion_r160617501
--- Diff: docs/dev/connectors/elasticsearch.md ---
@@ -440,36 +440,7 @@ For the execution of your Flink program, it is
recommended to build a
so
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5243#discussion_r160618041
--- Diff: flink-connectors/flink-connector-elasticsearch/pom.xml ---
@@ -93,6 +93,106 @@ under the License.
3
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/4150
On second thought, the change is actually a general improvement to the
packaging of the connector, so it would make sense to merge this to `master`
and `release-1.4` also.
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5138
I was waiting for any objections from others before moving ahead to merge
this.
If there are no other objections, will merge by the end of the week :)
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5171
+1, LGTM. Thanks @bowenli86.
Merging this ..
---
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5182#discussion_r160610346
--- Diff:
flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/internals/KinesisDataFetcher.java
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5182#discussion_r160610484
--- Diff:
flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/internals/KinesisDataFetcher.java
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5173#discussion_r160608176
--- Diff:
flink-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/util/serialization/JsonRowSerializationSchema.java
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5173#discussion_r160607315
--- Diff:
flink-connectors/flink-connector-kafka-0.8/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaConsumer081.java
---
@@ -27,6
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5173#discussion_r160608074
--- Diff:
flink-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/util/serialization/JsonRowDeserializationSchema.java
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5173#discussion_r160608041
--- Diff:
flink-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/util/serialization/JSONDeserializationSchema.java
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5173#discussion_r160607865
--- Diff:
flink-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/connectors/kafka/partitioner/FlinkKafkaPartitioner.java
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5173#discussion_r160607814
--- Diff:
flink-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/connectors/kafka/partitioner/FlinkFixedPartitioner.java
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5173#discussion_r160607372
--- Diff:
flink-connectors/flink-connector-kafka-0.8/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaProducer.java
---
@@ -31,6 +32,7
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5173#discussion_r160608269
--- Diff:
flink-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/util/serialization/KeyedSerializationSchema.java
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5173#discussion_r160607359
--- Diff:
flink-connectors/flink-connector-kafka-0.8/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaConsumer082.java
---
@@ -27,6
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5173#discussion_r160608223
--- Diff:
flink-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/util/serialization/KeyedDeserializationSchema.java
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5173#discussion_r160608063
--- Diff:
flink-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/util/serialization/JSONKeyValueDeserializationSchema.java
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5214
Good idea. I think we should also update the metrics doc to educate this
addition.
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5200
Regarding the race condition you mentioned:
hmm, I can't seem to exactly nail down the "and only first
`successfulCommits.inc()` can be omitted because of that" case you mentioned,
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5200
@pnowojski thanks a lot for your insightful review!
regarding the choice of composition or inheritance: actually, in the end I
think we should be leaning towards neither of both, and let
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5200#discussion_r160599433
--- Diff:
flink-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/connectors/kafka/internals/AbstractFetcher.java
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5200#discussion_r160598939
--- Diff:
flink-connectors/flink-connector-kafka-base/src/test/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaConsumerBaseTest.java
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5200#discussion_r16059
--- Diff:
flink-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaConsumerBase.java
---
@@ -559,6
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5200#discussion_r160598772
--- Diff:
flink-connectors/flink-connector-kafka-base/src/main/java/org/apache/flink/streaming/connectors/kafka/internals/AbstractFetcher.java
GitHub user tzulitai opened a pull request:
https://github.com/apache/flink/pull/5269
[FLINK-6004] [kinesis] Allow FlinkKinesisConsumer to skip
non-deserializable records
## What is the purpose of the change
This PR is based on #5268, which includes fixes to harden Kinesis
GitHub user tzulitai opened a pull request:
https://github.com/apache/flink/pull/5268
[FLINK-8398] [kinesis, tests] Harden KinesisDataFetcherTest unit tests
## What is the purpose of the change
Prior to this PR, many of the `KinesisDataFetcherTest` unit tests relied
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/4150
Merging to `release-1.3` ..
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/4150
@casidiablo thanks for the info!
I'll merge this for `release-1.3` then, and will keep an extra eye on
whether the problem still occurs for 1.4 / 1.5.
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/4150
@casidiablo did you mean that without applying this PR's patch, the current
master worked for you? Or you had to apply this patch in order for it to work?
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5193
Merging to `master` ..
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5179
Thanks a lot for the review. Merging this to `master` and `release-1.4` ..
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5201
Thanks for the review @pnowojski. I'm merging the last commit of this PR to
`master` and `release-1.4` ..
---
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r159724744
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/testutils/MockEnvironment.java
---
@@ -383,7 +383,10 @@ public void
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r159723523
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/util/SourceFunctionUtil.java
---
@@ -42,22 +41,32 @@
public class
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r159722857
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/util/AbstractStreamOperatorTestHarness.java
---
@@ -492,6 +503,10 @@ public void
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5201
The following 10 local Travis runs suggests that the stalling tests on the
`connectors` build no longer remains after this change:
- https://travis-ci.org/tzulitai/flink/builds/319936358
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r158401540
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/util/SourceFunctionUtil.java
---
@@ -42,22 +41,32 @@
public class
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r158398663
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/operators/windowing/WindowOperatorTest.java
---
@@ -823,39 +839,44
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r158400189
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/util/AbstractStreamOperatorTestHarness.java
---
@@ -492,6 +503,10 @@ public void
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r158397131
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/operators/testutils/TaskTestBase.java
---
@@ -145,19 +145,7 @@ public MemoryManager
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r158401883
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/api/functions/sink/TwoPhaseCommitSinkFunctionTest.java
---
@@ -179,9 +179,6
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r158400738
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/operators/windowing/WindowOperatorTest.java
---
@@ -98,7 +99,13
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r158399966
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/util/AbstractStreamOperatorTestHarness.java
---
@@ -492,6 +503,10 @@ public void
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r158395271
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/api/functions/sink/ContentDump.java
---
@@ -0,0 +1,134 @@
+/*
+ * Licensed
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r158394094
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/api/functions/sink/TwoPhaseCommitSinkFunctionTest.java
---
@@ -277,88 +263,67
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r158394946
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/api/functions/sink/ContentDump.java
---
@@ -0,0 +1,134 @@
+/*
+ * Licensed
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r158391433
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/api/functions/sink/ContentDump.java
---
@@ -0,0 +1,134 @@
+/*
+ * Licensed
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r158392202
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/api/functions/sink/ContentDump.java
---
@@ -0,0 +1,134 @@
+/*
+ * Licensed
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5193#discussion_r158394743
--- Diff:
flink-streaming-java/src/test/java/org/apache/flink/streaming/api/functions/sink/ContentDump.java
---
@@ -0,0 +1,134 @@
+/*
+ * Licensed
GitHub user tzulitai opened a pull request:
https://github.com/apache/flink/pull/5201
[FLINK-8283] [kafka] Stabalize FlinkKafkaConsumerBaseTest::testScaleUp()
## What is the purpose of the change
This PR sits upon other `FlinkKafkaConsumerBaseTest` changes from #5188
GitHub user tzulitai opened a pull request:
https://github.com/apache/flink/pull/5200
[FLINK-8306] [kafka] Introduce KafkaOffsetCommitter interface
## What is the purpose of the change
This PR is built upon the reworked `FlinkKafkaConsumerBaseTest` in #5188.
Prior
Github user tzulitai closed the pull request at:
https://github.com/apache/flink/pull/5189
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5189
It seems like that the mock issue on
`AbstractFetcher::commitInternalOffsetsToKafka` is not the root cause of the
test instabilities mentioned in FLINK-8283. It is rather just a separate issue
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5171#discussion_r158166090
--- Diff:
flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/util/AWSUtil.java
---
@@ -30,37 +30,44
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5171#discussion_r158164454
--- Diff:
flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/util/AWSUtil.java
---
@@ -30,37 +30,44
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5171#discussion_r158163793
--- Diff:
flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/util/AWSUtil.java
---
@@ -30,37 +30,44
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5182#discussion_r158160866
--- Diff:
flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/internals/ShardConsumer.java
---
@@ -96,6
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5182#discussion_r158159640
--- Diff: docs/monitoring/metrics.md ---
@@ -1293,6 +1293,29 @@ Thus, in order to infer the metric identifier:
+ Kinesis
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5182#discussion_r158161267
--- Diff:
flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/internals/ShardConsumer.java
---
@@ -96,6
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5182#discussion_r158161564
--- Diff:
flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/internals/ShardConsumer.java
---
@@ -96,6
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5191
This seems like a mistakenly opened PR.
Can you please close this, @czhxmz? Thanks!
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5187
This seems like a mistake. @laolang113 can you please close this PR? Thanks!
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5195
+1, LGTM
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5171
Ah, I see. The `AmazonKinesisClient` class is not deprecated, but the
constructors are.
Alright, thanks for the clarification. I'll find some time to revisit this
PR tomorrow.
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5181
Also FYI: The stalling tests seems to have been fixed (indirectly?) by
fixing the mocking issue. No failures have occurred anymore over 10 test local
Travis runs.
---
Github user tzulitai closed the pull request at:
https://github.com/apache/flink/pull/5181
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5181
Making the `AbstractFetcher::commitInternalOffsetsToKafka` non-final just
for the sake of testing using mocks really is not ideal.
I've opened a new PR #5189 that properly solves
GitHub user tzulitai opened a pull request:
https://github.com/apache/flink/pull/5189
[FLINK-8283] [kafka] Introduce KafkaOffsetCommitter interface
## What is the purpose of the change
This PR is built upon the reworked `FlinkKafkaConsumerBaseTest` in #5188.
The broken
GitHub user tzulitai opened a pull request:
https://github.com/apache/flink/pull/5188
[FLINK-8296] [kafka] Rework FlinkKafkaConsumerBaseTest to not rely on Java
reflection
## What is the purpose of the change
Prior to this PR, reflection was mainly used to inject mocks
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5181
I'm not yet certain this fix is the root cause of the stalling tests
mentioned in https://issues.apache.org/jira/browse/FLINK-8283. It's something I
stumbled across while investigating the failing
GitHub user tzulitai opened a pull request:
https://github.com/apache/flink/pull/5181
[FLINK-8283] [kafka] Fix mock verification on final method in
FlinkKafkaConsumerBaseTest
## What is the purpose of the change
Prior to this PR,
`FlinkKafkaConsumerBaseTest
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5178
+1, thanks for the fix.
Merging this ...
---
GitHub user tzulitai opened a pull request:
https://github.com/apache/flink/pull/5179
[FLINK-8260/8287] [kafka] Bunch of improvements to Kafka producer Javadocs
/ document
## What is the purpose of the change
This PR collects several improvements to the `FlinkKafkaProducer
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5171
Without the SDK upgrade, the API actually isn't yet deprecated yet, right?
It is only deprecated in a newer SDK version.
If so, I would prefer to not merge this until we actually try
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5121
Thanks @ankitiitb1069 @ggevay for the work and review.
The changes LGTM, minus my comment. I'll address my comments while merging
this ...
---
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5121#discussion_r157606329
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/source/SourceFunction.java
---
@@ -61,9 +61,9
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/5121#discussion_r157606375
--- Diff:
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/source/SourceFunction.java
---
@@ -61,9 +61,9
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5172
@suez1224 keep in mind, that contribution PRs should initially have one
commit with the commit message appropriately set (the title of the PR would be
a good commit message for your case).
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5172
cc @EronWright, gentle ping since you mentioned to include you in the
review :)
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5171
@bowenli86 I mean't the actual commit messages, not the PR title /
description.
The actual commit messages should follow our standard commit message
formats.
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5172
Thanks for the PR @suez1224.
There is a duplicate JIRA for this issue:
https://issues.apache.org/jira/browse/FLINK-8270.
Can you take a look at the suggestions explained in that JIRA
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5135
Thanks! LGTM.
Merging this ...
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5124
The PR has correct identification of what should be public and what is
internal, IMO.
However, as I also explained in #5138, I think we should not go with the
`@Public` annotation now
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5130
Merging this ..
---
401 - 500 of 1924 matches
Mail list logo
