Skip to content

[SPARK-6305][BUILD][FOLLOWUP] Remove log4j1 properties files #34941

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
viirya wants to merge 4 commits into from
Closed

[SPARK-6305][BUILD][FOLLOWUP] Remove log4j1 properties files #34941

viirya wants to merge 4 commits into from

Conversation

@viirya
Copy link
Member

@viirya viirya commented Dec 18, 2021 •
edited
Loading

What changes were proposed in this pull request?

This patch removes log4j1 properties files, rewrites log4j.properties.template and mention the migration in the migration guide.

Why are the changes needed?

After migrating from log4j1 to log4j2, we can remove log4j1 properties files.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing tests.

@SparkQA
Copy link

SparkQA commented Dec 18, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50825/

@pan3793
Copy link
Member

pan3793 commented Dec 18, 2021

conf/log4j.properties.template should be updated as well.

@viirya
Copy link
Member Author

viirya commented Dec 18, 2021

Yea, I will update it.

@pan3793
Copy link
Member

pan3793 commented Dec 18, 2021

There is a minor typo mentioned in #34935, please fix and close #34935 if you update conf/log4j.properties.template in this PR.

@SparkQA
Copy link

SparkQA commented Dec 18, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50825/

@SparkQA
Copy link

SparkQA commented Dec 18, 2021

Test build #146351 has finished for PR 34941 at commit ccdb259.

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

@SparkQA
Copy link

SparkQA commented Dec 18, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50839/

@SparkQA
Copy link

SparkQA commented Dec 18, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50839/

@SparkQA
Copy link

SparkQA commented Dec 18, 2021

Test build #146365 has finished for PR 34941 at commit d824ebf.

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

@viirya
Copy link
Member Author

viirya commented Dec 19, 2021

cc @dongjoon-hyun

dongjoon-hyun
dongjoon-hyun approved these changes Dec 19, 2021
View reviewed changes
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM

HyukjinKwon
HyukjinKwon reviewed Dec 19, 2021
View reviewed changes
@github-actions github-actions bot added the DOCS label Dec 19, 2021
@SparkQA
Copy link

SparkQA commented Dec 19, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50856/

@SparkQA
Copy link

SparkQA commented Dec 19, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50856/

@github-actions github-actions bot added the BUILD label Dec 19, 2021
@SparkQA
Copy link

SparkQA commented Dec 19, 2021

Test build #146381 has finished for PR 34941 at commit 260b0b2.

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

@SparkQA
Copy link

SparkQA commented Dec 19, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50857/

@SparkQA
Copy link

SparkQA commented Dec 19, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50857/

@SparkQA
Copy link

SparkQA commented Dec 20, 2021

Test build #146382 has finished for PR 34941 at commit 79cc6af.

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

@viirya
Copy link
Member Author

viirya commented Dec 20, 2021

Thanks. Merging to master.

@viirya viirya closed this in 34fb801 Dec 20, 2021
LuciferYang
LuciferYang reviewed Dec 20, 2021
View reviewed changes
conf/log4j2.properties.template
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
Copy link
Contributor

@LuciferYang LuciferYang Dec 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

log4j2 supports both properties and xml configurations. Do Spark prefer properties？ I found that the official documents of log4j2 are always show demos use xml

Copy link
Contributor

@beliefer beliefer Dec 20, 2021

Choose a reason for hiding this comment

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

It seems will follow the user's behavior.

@beliefer
Copy link
Contributor

beliefer commented Dec 20, 2021

It seems the migration makes break change.

@Ngone51
Copy link
Member

Ngone51 commented Dec 23, 2021

@viirya Did you miss this one: https://github.com/apache/spark/blob/master/core/src/main/resources/org/apache/spark/log4j-defaults.properties?

@viirya
Copy link
Member Author

viirya commented Dec 23, 2021

@Ngone51 Yea, it will be removed by #34965.

@Ngone51
Copy link
Member

Ngone51 commented Dec 23, 2021

Ok, good to know.

sumwale pushed a commit to TIBCOSoftware/snappy-spark that referenced this pull request Feb 9, 2022
@viirya
[SPARK-6305][BUILD][FOLLOWUP] Remove log4j1 properties files
83bcebb 
This patch removes log4j1 properties files, rewrites log4j.properties.template and mention the migration in the migration guide.

After migrating from log4j1 to log4j2, we can remove log4j1 properties files.

No

Existing tests.

Closes apache#34941 from viirya/remove_log4j_properties.

Authored-by: Liang-Chi Hsieh <viirya@gmail.com>
Signed-off-by: Liang-Chi Hsieh <viirya@gmail.com>
jiaoew1991 pushed a commit to Kyligence/spark that referenced this pull request Apr 2, 2022
@viirya
[SPARK-6305][BUILD][FOLLOWUP] Remove log4j1 properties files
77bc061 
### What changes were proposed in this pull request?

This patch removes log4j1 properties files, rewrites log4j.properties.template and mention the migration in the migration guide.

### Why are the changes needed?

After migrating from log4j1 to log4j2, we can remove log4j1 properties files.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Existing tests.

Closes apache#34941 from viirya/remove_log4j_properties.

Authored-by: Liang-Chi Hsieh <viirya@gmail.com>
Signed-off-by: Liang-Chi Hsieh <viirya@gmail.com>
kazuyukitanimura pushed a commit to kazuyukitanimura/spark that referenced this pull request Aug 10, 2022
@viirya
[SPARK-6305][BUILD][FOLLOWUP] Remove log4j1 properties files
2956a6b 
### What changes were proposed in this pull request?

This patch removes log4j1 properties files, rewrites log4j.properties.template and mention the migration in the migration guide.

### Why are the changes needed?

After migrating from log4j1 to log4j2, we can remove log4j1 properties files.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Existing tests.

Closes apache#34941 from viirya/remove_log4j_properties.

Authored-by: Liang-Chi Hsieh <viirya@gmail.com>
Signed-off-by: Liang-Chi Hsieh <viirya@gmail.com>
asiunov pushed a commit to ascend-io/spark that referenced this pull request Aug 25, 2022
@viirya @asiunov
[SPARK-6305][BUILD][FOLLOWUP] Remove log4j1 properties files
d302953 
This patch removes log4j1 properties files, rewrites log4j.properties.template and mention the migration in the migration guide.

After migrating from log4j1 to log4j2, we can remove log4j1 properties files.

No

Existing tests.

Closes apache#34941 from viirya/remove_log4j_properties.

Authored-by: Liang-Chi Hsieh <viirya@gmail.com>
Signed-off-by: Liang-Chi Hsieh <viirya@gmail.com>
(cherry picked from commit 34fb801)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LuciferYang LuciferYang LuciferYang left review comments

@beliefer beliefer beliefer left review comments

@HyukjinKwon HyukjinKwon HyukjinKwon approved these changes

@dongjoon-hyun dongjoon-hyun dongjoon-hyun approved these changes

Assignees

No one assigned

Labels

AVRO BUILD CORE DOCS DSTREAM GRAPHX KUBERNETES MESOS SPARK SHELL SQL STRUCTURED STREAMING YARN

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@viirya @SparkQA @pan3793 @beliefer @Ngone51 @LuciferYang @HyukjinKwon @dongjoon-hyun