Skip to content

[SPARK-3543] Write TaskContext in Java and expose it through a static accessor. #2425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 7 commits into from
Closed

[SPARK-3543] Write TaskContext in Java and expose it through a static accessor. #2425

wants to merge 7 commits into from

Conversation

@ScrapCodes
Copy link
Member

@ScrapCodes ScrapCodes commented Sep 17, 2014

No description provided.

rxin
rxin reviewed Sep 17, 2014
View reviewed changes
core/src/main/java/org/apache/spark/TaskContext.java Outdated
Copy link
Contributor

@rxin rxin Sep 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

u don't need to wrap this, do u?

Copy link
Member Author

@ScrapCodes ScrapCodes Sep 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean text wrap ?, yes in one line they are 114 chars.

@SparkQA
Copy link

SparkQA commented Sep 17, 2014

QA tests have started for PR 2425 at commit f716fd1.

  • This patch merges cleanly.

rxin
rxin reviewed Sep 17, 2014
View reviewed changes
core/src/main/java/org/apache/spark/TaskContext.java Outdated
Copy link
Contributor

@rxin rxin Sep 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space before {

@ScrapCodes
Copy link
Member Author

ScrapCodes commented Sep 17, 2014

@rxin One side question, Java8ApiSuite(s) don't compile, looks like we have been overlooking them for a while. May be we could just remove them ?

@SparkQA
Copy link

SparkQA commented Sep 17, 2014

QA tests have started for PR 2425 at commit edf945e.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 17, 2014

QA tests have finished for PR 2425 at commit f716fd1.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Sep 17, 2014

QA tests have started for PR 2425 at commit a7d5e23.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 17, 2014

QA tests have finished for PR 2425 at commit edf945e.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Sep 17, 2014

QA tests have finished for PR 2425 at commit a7d5e23.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

rxin
rxin reviewed Sep 17, 2014
View reviewed changes
core/src/main/java/org/apache/spark/TaskContext.java Outdated
Copy link
Contributor

@rxin rxin Sep 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int? why box?

@SparkQA
Copy link

SparkQA commented Sep 18, 2014

QA tests have started for PR 2425 at commit ee8bd00.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 18, 2014

QA tests have finished for PR 2425 at commit ee8bd00.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

pwendell
pwendell reviewed Sep 21, 2014
View reviewed changes
core/src/main/scala/org/apache/spark/scheduler/Task.scala
Copy link
Contributor

@pwendell pwendell Sep 21, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason to drop the naming for runningLocally? the correct style is to name all optional paremeters.

Copy link
Contributor

@rxin rxin Sep 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the parameter is defined in java it cannot be named

@pwendell
Copy link
Contributor

pwendell commented Sep 21, 2014

@ScrapCodes made another pass with some comments. Overall this is looking good

@pwendell
Copy link
Contributor

pwendell commented Sep 25, 2014

@ScrapCodes will you have time to address the feedback?

@SparkQA
Copy link

SparkQA commented Sep 26, 2014

QA tests have started for PR 2425 at commit 155bc3b.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 26, 2014

QA tests have finished for PR 2425 at commit 155bc3b.

  • This patch fails unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

AmplabJenkins commented Sep 26, 2014

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20854/

@ScrapCodes ScrapCodes force-pushed the SPARK-3543/withTaskContext branch from 155bc3b to 5f8555b Compare September 26, 2014 10:32
@SparkQA
Copy link

SparkQA commented Sep 26, 2014

QA tests have started for PR 2425 at commit 5f8555b.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 26, 2014

QA tests have finished for PR 2425 at commit 5f8555b.

  • This patch fails unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

AmplabJenkins commented Sep 26, 2014

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20855/

@ScrapCodes ScrapCodes force-pushed the SPARK-3543/withTaskContext branch from 5f8555b to 8ae414c Compare September 26, 2014 10:44
@SparkQA
Copy link

SparkQA commented Sep 26, 2014

QA tests have started for PR 2425 at commit 8ae414c.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 26, 2014

QA tests have finished for PR 2425 at commit 8ae414c.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

AmplabJenkins commented Sep 26, 2014

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20856/

@rxin
Copy link
Contributor

rxin commented Sep 27, 2014

Ok I'm merging this. I will push some minor cleanups after merging.

@asfgit asfgit closed this in 5e34855 Sep 27, 2014
rxin
rxin reviewed Sep 27, 2014
View reviewed changes
core/src/main/java/org/apache/spark/TaskContext.java
Copy link
Contributor

@rxin rxin Sep 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi all these should be primitives rather than boxed Integers/Longs/Booleans. I'm going to fix it in a separate pr.

This was referenced Sep 27, 2014
Closed
Closed
asfgit pushed a commit that referenced this pull request Sep 27, 2014
@rxin
[SPARK-3543] Clean up Java TaskContext implementation.
5b922bb 
This addresses some minor issues in #2425

Author: Reynold Xin <[email protected]>

Closes #2557 from rxin/TaskContext and squashes the following commits:

a51e5f6 [Reynold Xin] [SPARK-3543] Clean up Java TaskContext implementation.
@ScrapCodes ScrapCodes deleted the SPARK-3543/withTaskContext branch June 3, 2015 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ScrapCodes @SparkQA @pwendell @AmplabJenkins @rxin @shashank21j