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] Uses a temporary log4j.properties in HiveThriftServer2Test to ensure expected logging behavior #6493

Closed
wants to merge 2 commits into from

Conversation

liancheng
Copy link
Contributor

The HiveThriftServer2Test relies on proper logging behavior to assert whether the Thrift server daemon process is started successfully. However, some other jar files listed in the classpath may potentially contain an unexpected Log4J configuration file which overrides the logging behavior.

This PR writes a temporary log4j.properties and prepend it to driver classpath before starting the testing Thrift server process to ensure proper logging behavior.

cc @andrewor14 @yhuai

@SparkQA
Copy link

SparkQA commented May 29, 2015

Test build #33730 has finished for PR 6493 at commit b46ef0d.

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

@andrewor14
Copy link
Contributor

Does this mean the test results won't appear in unit-test.logs?

@yhuai
Copy link
Contributor

yhuai commented May 29, 2015

It should only affect the thrift server. Not the client side (those unit tests).

@andrewor14
Copy link
Contributor

I see, because it's adding it to the driver JVM, not to the test JVM.

@andrewor14
Copy link
Contributor

I just tried this patch out locally on top of #6441 and verified that the hive-thriftserver tests now pass.

@andrewor14
Copy link
Contributor

Merging into master 1.4 once tests pass.

@SparkQA
Copy link

SparkQA commented May 29, 2015

Test build #33743 has finished for PR 6493 at commit c489e0e.

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

asfgit pushed a commit that referenced this pull request May 29, 2015
…erver2Test to ensure expected logging behavior

The `HiveThriftServer2Test` relies on proper logging behavior to assert whether the Thrift server daemon process is started successfully. However, some other jar files listed in the classpath may potentially contain an unexpected Log4J configuration file which overrides the logging behavior.

This PR writes a temporary `log4j.properties` and prepend it to driver classpath before starting the testing Thrift server process to ensure proper logging behavior.

cc andrewor14 yhuai

Author: Cheng Lian <[email protected]>

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

c489e0e [Cheng Lian] Fixes minor Scala styling issue
b46ef0d [Cheng Lian] Uses a temporary log4j.properties in HiveThriftServer2Test to ensure expected logging behavior

(cherry picked from commit 4782e13)
Signed-off-by: Andrew Or <[email protected]>
@asfgit asfgit closed this in 4782e13 May 29, 2015
@liancheng liancheng deleted the override-log4j branch May 30, 2015 12:46
asfgit pushed a commit that referenced this pull request Jun 3, 2015
…erver2Test to ensure expected logging behavior

The `HiveThriftServer2Test` relies on proper logging behavior to assert whether the Thrift server daemon process is started successfully. However, some other jar files listed in the classpath may potentially contain an unexpected Log4J configuration file which overrides the logging behavior.

This PR writes a temporary `log4j.properties` and prepend it to driver classpath before starting the testing Thrift server process to ensure proper logging behavior.

cc andrewor14 yhuai

Author: Cheng Lian <[email protected]>

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

c489e0e [Cheng Lian] Fixes minor Scala styling issue
b46ef0d [Cheng Lian] Uses a temporary log4j.properties in HiveThriftServer2Test to ensure expected logging behavior
@andrewor14
Copy link
Contributor

OK, I re-merged this in 1.4 since the follow up fix for java 6 is up.

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 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
@andrewor14
Copy link
Contributor

@liancheng seems like this is needed for branch-1.3 for #6602. Could you open a branch against that one?

@andrewor14
Copy link
Contributor

Actually I did it myself in #6602. Never mind.

jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request Jun 12, 2015
…erver2Test to ensure expected logging behavior

The `HiveThriftServer2Test` relies on proper logging behavior to assert whether the Thrift server daemon process is started successfully. However, some other jar files listed in the classpath may potentially contain an unexpected Log4J configuration file which overrides the logging behavior.

This PR writes a temporary `log4j.properties` and prepend it to driver classpath before starting the testing Thrift server process to ensure proper logging behavior.

cc andrewor14 yhuai

Author: Cheng Lian <[email protected]>

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

c489e0e [Cheng Lian] Fixes minor Scala styling issue
b46ef0d [Cheng Lian] Uses a temporary log4j.properties in HiveThriftServer2Test to ensure expected logging behavior
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
…erver2Test to ensure expected logging behavior

The `HiveThriftServer2Test` relies on proper logging behavior to assert whether the Thrift server daemon process is started successfully. However, some other jar files listed in the classpath may potentially contain an unexpected Log4J configuration file which overrides the logging behavior.

This PR writes a temporary `log4j.properties` and prepend it to driver classpath before starting the testing Thrift server process to ensure proper logging behavior.

cc andrewor14 yhuai

Author: Cheng Lian <[email protected]>

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

c489e0e [Cheng Lian] Fixes minor Scala styling issue
b46ef0d [Cheng Lian] Uses a temporary log4j.properties in HiveThriftServer2Test to ensure expected logging behavior
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
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