Skip to content
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.

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

[SQL] [TEST] [MINOR] Follow-up of PR #6493, use Guava API to ensure Java 6 friendliness #6547

Closed
wants to merge 2 commits into from

Conversation

liancheng
Copy link
Contributor

This is a follow-up of PR #6493, which has been reverted in branch-1.4 because it uses Java 7 specific APIs and breaks Java 6 build. This PR replaces those APIs with equivalent Guava ones to ensure Java 6 friendliness.

cc @andrewor14 @pwendell, this should also be back ported to branch-1.4.

@andrewor14
Copy link
Contributor

lgtm merging once tests pass

@@ -29,6 +27,8 @@ import scala.concurrent.{Await, Promise}
import scala.sys.process.{Process, ProcessLogger}
import scala.util.{Random, Try}

import com.google.common.base.Charsets
Copy link
Member

Choose a reason for hiding this comment

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

Could you use com.google.common.base.Charsets.UTF_8? So that after upgrading to Java 7, we can simply update the imports without touching the codes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please elaborate on this? I don't quite get what do you mean...

Copy link
Member

Choose a reason for hiding this comment

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

I mean if importing com.google.common.base.Charsets.UTF_8 here, you can use UTF_8 directly in the codes. And when we decide to switch to java.nio.charset.StandardCharsets.UTF_8, we only need to update the import.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see, makes sense, thanks!

@SparkQA
Copy link

SparkQA commented Jun 1, 2015

Test build #33876 has finished for PR 6547 at commit 72da795.

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

@liancheng
Copy link
Contributor Author

Addressed @zsxwing's comment, will merge this to master and branch-1.4 once Jenkins passes.

@SparkQA
Copy link

SparkQA commented Jun 2, 2015

Test build #33972 has finished for PR 6547 at commit c900cfd.

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

@andrewor14
Copy link
Contributor

master 1.4 thanks cheng

asfgit pushed a commit that referenced this pull request Jun 3, 2015
…ava 6 friendliness

This is a follow-up of PR #6493, which has been reverted in branch-1.4 because it uses Java 7 specific APIs and breaks Java 6 build. This PR replaces those APIs with equivalent Guava ones to ensure Java 6 friendliness.

cc andrewor14 pwendell, this should also be back ported to branch-1.4.

Author: Cheng Lian <[email protected]>

Closes #6547 from liancheng/override-log4j and squashes the following commits:

c900cfd [Cheng Lian] Addresses Shixiong's comment
72da795 [Cheng Lian] Uses Guava API to ensure Java 6 friendliness

(cherry picked from commit 5cd6a63)
Signed-off-by: Andrew Or <[email protected]>
@asfgit asfgit closed this in 5cd6a63 Jun 3, 2015
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request Jun 12, 2015
…sure Java 6 friendliness

This is a follow-up of PR apache#6493, which has been reverted in branch-1.4 because it uses Java 7 specific APIs and breaks Java 6 build. This PR replaces those APIs with equivalent Guava ones to ensure Java 6 friendliness.

cc andrewor14 pwendell, this should also be back ported to branch-1.4.

Author: Cheng Lian <[email protected]>

Closes apache#6547 from liancheng/override-log4j and squashes the following commits:

c900cfd [Cheng Lian] Addresses Shixiong's comment
72da795 [Cheng Lian] Uses Guava API to ensure Java 6 friendliness
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
…sure Java 6 friendliness

This is a follow-up of PR apache#6493, which has been reverted in branch-1.4 because it uses Java 7 specific APIs and breaks Java 6 build. This PR replaces those APIs with equivalent Guava ones to ensure Java 6 friendliness.

cc andrewor14 pwendell, this should also be back ported to branch-1.4.

Author: Cheng Lian <[email protected]>

Closes apache#6547 from liancheng/override-log4j and squashes the following commits:

c900cfd [Cheng Lian] Addresses Shixiong's comment
72da795 [Cheng Lian] Uses Guava API to ensure Java 6 friendliness
@liancheng liancheng deleted the override-log4j branch July 9, 2015 21:35
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.

4 participants