Skip to content

[SPARK-1470][SPARK-1842] Use the scala-logging wrapper instead of the directly sfl4j api - #1369

Closed
witgo wants to merge 2 commits into
apache:masterfrom
witgo:SPARK-1470_new
Closed

[SPARK-1470][SPARK-1842] Use the scala-logging wrapper instead of the directly sfl4j api#1369
witgo wants to merge 2 commits into
apache:masterfrom
witgo:SPARK-1470_new

Conversation

@witgo

Copy link
Copy Markdown
Contributor

No description provided.

@SparkQA

Copy link
Copy Markdown

QA tests have started for PR 1369. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16552/consoleFull

@SparkQA

Copy link
Copy Markdown

QA results for PR 1369:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16552/consoleFull

@witgo

Copy link
Copy Markdown
ContributorAuthor

Jenkins, test this please.

@SparkQA

Copy link
Copy Markdown

QA tests have started for PR 1369. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16556/consoleFull

@SparkQA

Copy link
Copy Markdown

QA results for PR 1369:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16556/consoleFull

@SparkQA

Copy link
Copy Markdown

QA tests have started for PR 1369. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16564/consoleFull

@srowen

Copy link
Copy Markdown
Member

Wasn't this already decided against in #332 and again #1208 ? or is this not another PR for https://issues.apache.org/jira/browse/SPARK-1470 ?

@witgo

Copy link
Copy Markdown
ContributorAuthor

#332 can't automatic test .
#1208 was messing up and I do not know how to solve . 😓

@witgowitgo changed the title Use the scala-logging wrapper instead of the directly sfl4j api[SPARK-1470,SPARK-1842] Use the scala-logging wrapper instead of the directly sfl4j apiJul 11, 2014
@SparkQA

Copy link
Copy Markdown

QA results for PR 1369:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16564/consoleFull

@SparkQA

Copy link
Copy Markdown

QA tests have started for PR 1369. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16679/consoleFull

@SparkQA

Copy link
Copy Markdown

QA results for PR 1369:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16679/consoleFull

@witgowitgo changed the title [SPARK-1470,SPARK-1842] Use the scala-logging wrapper instead of the directly sfl4j api[SPARK-1470][SPARK-1842] Use the scala-logging wrapper instead of the directly sfl4j apiAug 1, 2014
@SparkQA

Copy link
Copy Markdown

QA tests have started for PR 1369. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17639/consoleFull

@SparkQA

Copy link
Copy Markdown

QA results for PR 1369:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17639/consoleFull

@SparkQA

Copy link
Copy Markdown

QA tests have started for PR 1369. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17640/consoleFull

@SparkQA

Copy link
Copy Markdown

QA results for PR 1369:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17640/consoleFull

@witgo

witgo commented Aug 1, 2014

Copy link
Copy Markdown
ContributorAuthor

Jenkins, retest this please.

@SparkQA

Copy link
Copy Markdown

QA tests have started for PR 1369. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17651/consoleFull

@SparkQA

Copy link
Copy Markdown

QA results for PR 1369:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17651/consoleFull

@srowen

Copy link
Copy Markdown
Member

@witgo Some of the failure was just Jenkins acting up again (what could be behind the "Address already in use" suddenly? for all of these tests), but there is a MIMA failure at the end as well. If the API changes are OK they will have to be excluded from the MIMA check to pass.

asfgit pushed a commit that referenced this pull request Aug 1, 2014
`breeze 0.8.1` dependent on `scala-logging-slf4j 2.1.1` The relevant code on #1369
Author: witgo <witgo@qq.com>
Closes#940 from witgo/breeze-8.0.1 and squashes the following commits:
65cc65e [witgo] update breeze to version 0.8.1
@witgo

witgo commented Aug 1, 2014

Copy link
Copy Markdown
ContributorAuthor

@srowen Thank you for your comments. And how to modify GenerateMIMAIgnore file to exclude a method? An example?

@SparkQA

Copy link
Copy Markdown

QA tests have started for PR 1369. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17683/consoleFull

@srowen

Copy link
Copy Markdown
Member

@witgo have a look at project/MimaExcludes.scala. Under the section starting with case v if v.startsWith("1.1") => you can add a Seq() with all the excludes necessary for your PR. The error messages tells you what you need to add.

Of course, the more important question, are those API changes on purpose and are they OK?

@SparkQA

Copy link
Copy Markdown

QA results for PR 1369:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17683/consoleFull

@witgo

witgo commented Aug 2, 2014

Copy link
Copy Markdown
ContributorAuthor

Jenkins, test this please.

@SparkQA

Copy link
Copy Markdown

QA tests have started for PR 1369. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17738/consoleFull

@SparkQA

Copy link
Copy Markdown

QA results for PR 1369:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17738/consoleFull

@witgo

witgo commented Aug 2, 2014

Copy link
Copy Markdown
ContributorAuthor

Jenkins, test this please.

@SparkQA

Copy link
Copy Markdown

QA tests have started for PR 1369. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17748/consoleFull

@SparkQA

Copy link
Copy Markdown

QA results for PR 1369:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17748/consoleFull

@pwendell

Copy link
Copy Markdown
Contributor

Hey on this one - it's helpful to see what this looks like - but my instinct is actually to move away from scala-logging entirely. We can upgrade ourselves, but all that does is force our users downstream to have to upgrade (even if they aren't using ml stuff at all), causing the same annoying headache as breeze is causing us. I'd prefer to just remove the (now very small) use of scala-logging and just continue to use the slf4j API directly. The mian benefit of scala-logging is the use of compile time macro's for some performance gain, but we've never in the entire history of Spark had any performance issues related to logging.

@pwendell

Copy link
Copy Markdown
Contributor

I accidentally merged this in lieu of another patch. The merge has been reverted.

@asfgitasfgit closed this in adc8303Aug 2, 2014
@witgo

witgo commented Aug 2, 2014

Copy link
Copy Markdown
ContributorAuthor

How to re-open this ?

@witgo
witgo deleted the SPARK-1470_new branch August 2, 2014 10:26
@witgo
witgo restored the SPARK-1470_new branch August 2, 2014 10:26
@witgo
witgo deleted the SPARK-1470_new branch August 2, 2014 10:26
@witgo
witgo restored the SPARK-1470_new branch August 2, 2014 10:26
@witgo
witgo deleted the SPARK-1470_new branch August 2, 2014 15:23
@witgo

witgo commented Aug 2, 2014

Copy link
Copy Markdown
ContributorAuthor

@pwendell#1208 fits your idea?

xiliu82 pushed a commit to xiliu82/spark that referenced this pull request Sep 4, 2014
`breeze 0.8.1` dependent on `scala-logging-slf4j 2.1.1` The relevant code on apache#1369
Author: witgo <witgo@qq.com>
Closesapache#940 from witgo/breeze-8.0.1 and squashes the following commits:
65cc65e [witgo] update breeze to version 0.8.1
xiliu82 pushed a commit to xiliu82/spark that referenced this pull request Sep 4, 2014
… directly sfl4j api
Author: GuoQiang Li <witgo@qq.com>
Closesapache#1369 from witgo/SPARK-1470_new and squashes the following commits:
66a1641 [GuoQiang Li] IncompatibleResultTypeProblem
73a89ba [GuoQiang Li] Use the scala-logging wrapper instead of the directly sfl4j api.
Sign up for freeto 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

@witgo@SparkQA@srowen@pwendell