Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-142272588
@HuangWHWHW No problem, we all learn all the time.
It would only help to review and merge pull requests if the style follows
more the Java best practices. It is
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-141904245
Hi, very sorry for bothering again.
Since two weeks passed, do you have some time to review this PR recently?
Will greatly appreciate it:)
---
If your
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-142173138
@StephanEwen
Hi, I'm very sorry for the poor Java style of mine.
And many thanks for your rework.I did a full review about your new fixes
and get the points.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-141952198
Looks good now, will 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
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-142014348
I reworked this quite heavily during merging. There were a lot of issues
that were against good Java style:
- Variables in the classes, rather than in methods
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1030
---
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 HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-140079694
Sorry for misunderstading.
I have changed the `LOG.error(e)` to `LOG.error(e.getMessage())` now.
---
If your project is set up for it, you can reply to this
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-138841521
I think this looks good, except for the comment with the final variable for
the lock.
One more comment: When concatenating strings, avoid constructs like `"
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-138842285
@tillrohrmann
Ah,sorry for bothering.
It doesn't matter.
Just I thought I did something wrong in the community.
:-D
---
If your project is set up
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-138836978
Sorry, @HuangWHWHW, currently we're really busy. I'll try to review your PR
once I find a free minute.
---
If your project is set up for it, you can reply to this
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-138843852
No worries. We are simply overloaded right now. Many hard features under
development, and many pull requests being opened.
---
If your project is set up for it,
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-138832801
@StephanEwen
@tillrohrmann
Hallo?
---
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 StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r39020317
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-139101747
Sorry for careless.
I forget to change the git global user.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-139104928
I removed the toString() method and change the lock to `private final
SerializableObject lock`.
---
If your project is set up for it, you can reply to this email
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-137747552
@StephanEwen
@tillrohrmann
Hi,
I get the CI to rerun.
Any new comment?
---
If your project is set up for it, you can reply to this email and have
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-136702507
@StephanEwen
Hi, I take changes for your comments but the synchronized (lock) in
SocketClientSink.java.
Need I change this to Thread.sleep()?
---
If your
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r38408542
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r38408571
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r38408757
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r38410693
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r38410820
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r38410780
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-136285097
@tillrohrmann
Hi,very sorry for disturbing.
I have changed the PR as your comments and passed the CI.
It will be thankful if you can take a look.
---
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-135630392
The method notifyAll() maybe get a bug in my test file that sometimes it
will be called before the wait().
So the method wait() will get stuck.
Then I change
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-134822139
@tillrohrmann
Hi, I fix the conflict and get the CI rerun.
Would you please to take a look about my new changes?
Whether there will be some new comments?
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-134513103
@tillrohrmann
Hi, I take a new fix.
But this:
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-134577603
@tillrohrmann
Ok, I will do a update in my branch.
But I cannot go to travis since it is blocked in China.
---
If your project is set up for it, you can
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-134573477
It should be re-triggered every time you push something new to your branch.
For your local CI you should be able to manually restart a build by going
to
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37745343
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37748350
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
@@
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37748875
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37745586
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37748000
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
@@
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37747966
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
@@
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37748510
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
@@
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37744884
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37744849
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37744808
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37744939
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37744832
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37750633
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37750694
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37751723
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37752541
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37752742
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37745169
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37744540
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-134175239
Hi @HuangWHWHW, I had some comments concerning the test cases.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37748110
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
@@
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37748179
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
@@
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-134196700
Hi,
Thank you.
I`ll update a new 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
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37745480
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37745469
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37748764
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37751549
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37751617
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-134107315
@tillrohrmann
Hi,
I take a new fix and add a test for retry success.
Would you please to take a look?
Thank you.
BTW:Why does not the CI
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37824613
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
@@
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37824211
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
@@
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37823131
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
@@
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37643848
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
@@
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37617867
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37618025
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37598999
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37599021
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37599157
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
@@
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-133238105
Could we also add a test case where we test that the SocketClientSink can
reconnect against a newly opened socket after it has been closed? This would be
great.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37563984
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-133111629
Hi @HuangWHWHW, I had some minor comments. Could we also add a test case
where we test that the `SocketClientSink` can reconnect against a newly opened
socket
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37562745
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37563493
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37562991
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37563275
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37563322
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/api/functions/sink/SocketClientSink.java
---
@@
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1030#discussion_r37564115
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/test/java/org/apache/flink/streaming/api/functions/sink/SocketClientSinkTest.java
---
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1030#issuecomment-132508921
Add tests for retry 10 times and 0.
---
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
GitHub user HuangWHWHW opened a pull request:
https://github.com/apache/flink/pull/1030
[FLINK-2536][streaming]add a re-connect for socket sink
add a re-connect in function invoke() when it throws exception.
You can merge this pull request into a Git repository by running:
$
78 matches
Mail list logo
