Skip to content

Conversation

@ChengXiangLi
Copy link

No description provided.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

Copy link
Member

Choose a reason for hiding this comment

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

Wrong indentation.

@ScrapCodes
Copy link
Member

It might be good to add a test suite for this in JavaAPISuite.java.

@pwendell
Copy link
Contributor

pwendell commented Sep 1, 2014

@ChengXiangLi could you describe a bit more what the context is being used for? This is an unstable API so I'm a bit hesitant to expose this in its current form. It would be better to look at exactly what Hive needs from this interface and see if we can come up with a stable interface for it.

@ChengXiangLi
Copy link
Author

Hi, @pwendell , For several Hive features, such as HIVE-7843 and HIVE-7627, Hive use partition id to distinct tasks write files in HDFS, no other dependency on task context found currently.

Copy link
Contributor

Choose a reason for hiding this comment

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

can we remove this one? I don't think it is needed for Hive, is it?

@rxin
Copy link
Contributor

rxin commented Sep 2, 2014

@JoshRosen can you also take a look at this. It is pretty short, but it is about the java api.

@rxin
Copy link
Contributor

rxin commented Sep 4, 2014

Jenkins, ok to test.

@JoshRosen
Copy link
Contributor

This looks fine to me, especially since it adds Java tests.

@SparkQA
Copy link

SparkQA commented Sep 4, 2014

QA tests have started for PR 2194 at commit 882b82e.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 4, 2014

QA tests have finished for PR 2194 at commit 882b82e.

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

@ChengXiangLi
Copy link
Author

we hit the binary incompatibilities error here, i already annotated new added methods as DeveloperApi, do i miss something here?

@rxin
Copy link
Contributor

rxin commented Sep 4, 2014

@ScrapCodes does mima check not exclude developer apis?

@ScrapCodes
Copy link
Member

I am looking at this. Mima check should have excluded those methods.

@ScrapCodes
Copy link
Member

@rxin There is a reason and (workaround type of)fix for this on #2285.

@SparkQA
Copy link

SparkQA commented Sep 5, 2014

Can one of the admins verify this patch?

@rxin
Copy link
Contributor

rxin commented Sep 10, 2014

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Sep 11, 2014

QA tests have started for PR 2194 at commit 882b82e.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 11, 2014

QA tests have finished for PR 2194 at commit 882b82e.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

you can't have default argument values in Java

@ChengXiangLi
Copy link
Author

We still hit API incompatibilities error as #2285 is not finished yet.

@rxin
Copy link
Contributor

rxin commented Sep 16, 2014

Jenkins, test this please.

@SparkQA
Copy link

SparkQA commented Sep 16, 2014

QA tests have started for PR 2194 at commit a5b7b41.

  • This patch merges cleanly.

@pwendell
Copy link
Contributor

I proposed a slightly different approach to this here:
https://issues.apache.org/jira/browse/SPARK-3543

This would remove the need for special methods xWithContext.

@SparkQA
Copy link

SparkQA commented Sep 16, 2014

QA tests have finished for PR 2194 at commit a5b7b41.

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

@rxin
Copy link
Contributor

rxin commented Sep 27, 2014

I pushed a commit to close this one in favor of #2425

@asfgit asfgit closed this in a3feaf0 Sep 27, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants