Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1386
---
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 StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-162467645
Thanks for addressing the comments.
+1, will merge this...
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user sachingoel0101 commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-162463009
Travis passes. Should be good to merge.
This also resolves FLINK-2524, so the commit message should reflect that
too.
---
If your project is set up for it, y
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1386#discussion_r46776269
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/operators/BatchTask.java
---
@@ -1150,8 +1149,8 @@ else if (index < 0 || index >=
this.dr
Github user sachingoel0101 commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-162345554
@StephanEwen thanks for the review. I have addressed all but one of your
comments.
---
If your project is set up for it, you can reply to this email and have yo
Github user sachingoel0101 commented on a diff in the pull request:
https://github.com/apache/flink/pull/1386#discussion_r46775517
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/operators/BatchTask.java
---
@@ -1150,8 +1149,8 @@ else if (index < 0 || index >=
this
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-162334713
All in all, pretty good change. Makes many parts of the code nicer. Also
good java docs and comments!
Comments were added inline. Nothing major, few details.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1386#discussion_r46773807
--- Diff:
flink-tests/src/test/java/org/apache/flink/test/misc/AttemptNumberITCase.java
---
@@ -0,0 +1,70 @@
+/*
--- End diff --
In orde
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1386#discussion_r46773693
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java ---
@@ -120,16 +121,9 @@
/** The execution attempt of the par
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1386#discussion_r46773689
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java ---
@@ -240,14 +234,13 @@ public Task(TaskDeploymentDescriptor tdd,
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1386#discussion_r46773676
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/operators/BatchTask.java
---
@@ -1150,8 +1149,8 @@ else if (index < 0 || index >=
this.dr
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1386#discussion_r46773630
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/broadcast/BroadcastVariableMaterialization.java
---
@@ -178,9 +178,9 @@ private boolean de
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1386#discussion_r46773625
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/broadcast/BroadcastVariableMaterialization.java
---
@@ -85,9 +85,9 @@ public void material
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1386#discussion_r46773363
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/functions/RuntimeContext.java
---
@@ -65,6 +65,22 @@
int getIndexOfThisSubtask();
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1386#discussion_r46773342
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/TaskInfo.java ---
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundati
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-162331160
I am adding a number of comments. When you address them, please do not
squash commits, so we can review just the changes separately.
---
If your project is set up f
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-162330497
Checking this out...
---
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 sachingoel0101 commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-162325866
@StephanEwen can you take a look again? I have introduced the `TaskInfo`
object in the second commit, and we can squash them before merging.
---
If your project
Github user sachingoel0101 commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-161921756
@StephanEwen you're absolutely right. It will most certainly do that.
I will push both things in a single commit and file an additional jira for
introducing th
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-161608168
As far as I can see from these changes, introducing a `TaskInfo` will
completely subsume this change here and change every line again.
You are right, usually
Github user sachingoel0101 commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-161449824
Yes. I was planning to start working on that after this. Since it's
preferable to fix different jiras in different commits, it'd be good if I
can base my wo
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-161286037
This generally looks good and pretty straight forward.
As such it is actually good to merge.
I remember that a while back, we were discussing to create a
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-160610157
I'll shepherd this pull request in...
---
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 sachingoel0101 commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-159853758
I'll wait for Stephan to review this then.
---
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 rmetzger commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-159662427
Hey @sachingoel0101, I think @StephanEwen is the best contact person here.
Since this affects a very important interface we shouldn't make any mistakes ;)
---
If your
Github user sachingoel0101 commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-159656251
Hi @rmetzger, can you take another look? I need to base some further work
on this [related to cleaning up passing of task name, index, etc.].
---
If your project
Github user sachingoel0101 commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-158408532
@rmetzger I have added a test case to verify the functionality.
---
If your project is set up for it, you can reply to this email and have your
reply appear on Gi
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/1386#issuecomment-158118963
Can you maybe extend a test which is restarting a job multiple times and
check there that the attempt number is set properly?
---
If your project is set up for it, you
GitHub user sachingoel0101 opened a pull request:
https://github.com/apache/flink/pull/1386
[FLINK-2488] Expose Attempt Number in RuntimeContext
Passes the attempt number all the way from `TaskDeploymentDescriptor` to
the `RuntimeContext`.
Small thing I want to confirm: For `Ru
29 matches
Mail list logo
