Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/517#issuecomment-34492639
@mridulm - we are adding checking for line lengths and some basic checks...
also many IDE's can be configured to enforce style guidelines.
GitHub user pwendell opened a pull request:
https://github.com/apache/incubator-spark/pull/560
[WIP] SPARK-1067: Default log4j initialization causes errors for those not
using log4j
To fix this - we add a check when initializing log4j.
You can merge this pull request into a Git
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/563#issuecomment-34553608
Jenkins add to whitelist.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/552#issuecomment-34553596
Ah sorry forgot to merge. I just did so it should show up here within an
hour.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/563#issuecomment-34553612
Jenkins, test this please.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/557#issuecomment-34553870
@ScrapCodes Words cannot express my elation at having this patch. I noticed
there are still style errors. Did you want me to merge this as-is and then you
will
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/557#issuecomment-34554122
Hey @ScrapCodes I noticed the size of indent is inconsistent. The rule is
to always use 2 spaces. If you are breaking initialization of a code block
(e.g. a
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/561#issuecomment-34554306
Good catch. Jenkins, test this please.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/550#issuecomment-34554613
@schmit Mind adding a JIRA for this?
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/454#issuecomment-34554940
Seems reasonable to me, I'll merge this.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/561#issuecomment-34555972
Thanks for this fix, I merged it into master and 0.9.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/563#issuecomment-34559476
@martinjaggi can you rebase this now?
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/564#issuecomment-34560617
@rezazadeh Mind adding a JIRA for this?
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/542#issuecomment-34560733
Per dev list discussion I am going to merge this. Thanks mark!
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/564#issuecomment-34563275
@rezazadeh We need to track all features with JIRA's it's an Apache
requirement.
GitHub user pwendell opened a pull request:
https://github.com/apache/incubator-spark/pull/565
SPARK-1066: Add developer scripts to repository.
These are some developer scripts I've been maintaining in a separate public
repo. This patch adds them to the Spark repository so the
Github user pwendell closed the pull request at:
https://github.com/apache/incubator-spark/pull/565
Github user pwendell closed the pull request at:
https://github.com/apache/incubator-spark/pull/560
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/557#issuecomment-34580972
Prashant - thanks a ton! I've merged this into master.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/472#issuecomment-34581663
Hey @CodingCat - I'd like to get more feedback from other users of this
feature before merging this in. Once we add an option we need to continue to
su
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/551#issuecomment-34581811
@rxin look good to you?
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/567#issuecomment-34582688
@ScrapCodes Looks good to me! If you can re-base this on your existing
patch I will merge it. Also can you add an Apache header to the xml file?
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/526#issuecomment-34585015
@rxin Hm seems like this didn't link up correctly either, wonder if this is
because the "Closes XX" gets broken in the way that github renders th
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/526#issuecomment-34585080
@rxin - ya I just checked. Kay's pull request had the same issue. I'll
change the script around to fix this.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/569#issuecomment-34586150
/cc @rxin
GitHub user pwendell opened a pull request:
https://github.com/apache/incubator-spark/pull/569
Fixes bug where merges won't close associated pull request.
Previously we added "Closes #XX" in the title. Github will sometimes
linbreak the title in a way that cau
Github user pwendell closed the pull request at:
https://github.com/apache/incubator-spark/pull/569
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/526#issuecomment-34590172
@tgravescs mind closing this
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/570#issuecomment-34605097
@prb the changes you are referring to are ripping out direct use of log4j
there... right?
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/560#issuecomment-34606359
FYI I reverted in both branches this because it caused a maven build error.
Also it was a WIP and @rxin merged it by accident.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/570#issuecomment-34606918
FYI #560 had a build issue so I had to revert it. But we'll fix that up and
get it merged soon into 0.9 as a fix.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/570#issuecomment-34606938
@prb - understood that this maintains the status quo otherwise, I hadn't
looked closely at it.
GitHub user pwendell opened a pull request:
https://github.com/apache/incubator-spark/pull/573
Default log4j initialization causes errors for those not using log4j
This is a re-do of #560 which got merged too soon. There are some maven
build issues we need to fix before merging
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/571#issuecomment-34607216
Jenkins, add to whitelist. Jenkins, test this please.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/573#issuecomment-34665107
@srowen There is a guard in the form of checking that slf4j is bound to
log4j... is there any other guard you can think of? I think this means this
should only
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/573#issuecomment-34698102
The issue is a user reported problems when they were writing from slf4j to
Logback, they had log4j-over-slf4j on the classpath, and this initialization
code
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/522#issuecomment-34838973
Jenkins, test this please. This was reviewed previously as #121 by matei. I
think it's good to go.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/522#issuecomment-34839244
@bijaybisht this is out of date with master - mind bringing it up to date?
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/584#issuecomment-34843361
Jenkins, test this please.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/584#issuecomment-34843366
LGTM pending tests.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/590#issuecomment-34945973
btw - I think the long term solution here is to have a script inside of the
spark repo that jenkins calls to run tests (./dev/run-tests) that way as we
evolve
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/574#issuecomment-34946324
Hey @ash211 this is an improvement! Let's just remove the "Merge pull
request..." as it is now redundant anyways with other information. Instea
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/574#issuecomment-34952393
Jenkins, retest this please.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/574#issuecomment-34952676
Andrew - looks great. Thanks for contributing this. I think the test error
was unrelated... pending successful tests LGTM.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/592#issuecomment-34952990
@aarondav @rxin we should modify Jenkins now so if this script is present,
it calls it :)
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/584#issuecomment-34953539
Jenkins, test this please.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/568#issuecomment-34953913
@bijaybisht Could you explain somewhere what the bug is that this is
fixing? The jira references "spark.driver.host" but in theory those should ha
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/593#issuecomment-34954387
LGTM
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/574#issuecomment-34954401
Merged (guess how!). Thanks Andrew
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/584#issuecomment-34954968
Thanks I merged this into master and 0.9.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/568#issuecomment-34955035
LGTM
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/568#issuecomment-34955031
@ScrapCodes ah I see - this is changing the system properties iterator to a
spark conf iterator. Makes sense.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/568#issuecomment-34955056
@aarondav pick this into master as well when you merge it.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/594#issuecomment-35004540
/cc @tdas
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/568#issuecomment-35004505
Jenkins, test this please.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/580#issuecomment-35004818
Jenkins, test this please. cc/ @ankurdave
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/468#issuecomment-35007789
Jenkins, test this please.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/596#issuecomment-35023537
Hey @tsdeng - this request is against branch 0.8, but this is a feature and
not a bug fix, so this should be against master.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/595#issuecomment-35023583
LGTM as well... I'll merge this thanks!
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/595#issuecomment-35023676
Merged, thanks
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/595#issuecomment-35024028
I put this in master in addition to branch 0.9
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/576#issuecomment-35034019
Hey there - this is interesting but I don't think it's something that we
should put inside of SparkContext. It's nice code as an example tho
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/598#issuecomment-35034124
@shivaram sorry for being lame and not doing this earlier... appreciate
you sending a PR.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/468#issuecomment-35058261
Hey @RongGu I did a pretty thorough review - thanks for adding this!
In the code I put a bunch of lower level comments. Here is some high level
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/468#issuecomment-35058467
Hey @alig re your comment about the documentation - it might make more
sense to integrate Tachyon in the existing pages that cover storage and
provisioning (I
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/600#issuecomment-35058750
Jenkins, test this please.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/402#issuecomment-35115847
@ScrapCodes as discussed earlier if you could add either tests or explicit
typing for the accessor methods that would be great.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/332#issuecomment-35134709
@hsaputra @tgravescs I think it's fine if, for now, this hard codes the
authentication mechanism (based on the description it seems the proposal is to
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/602#issuecomment-35138743
We can't accept this patch because it will regress behavior for existing
users. See this JIRA for a proposal relating to SPARK_MEM:
https://
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/585#issuecomment-35139456
Hey @ScrapCodes - this is a great start! To give some background, we
actually want to use this and reject pull requests that cause Mima errors. That
means we
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/603#issuecomment-35316947
@mateiz @nicklan - I think it's fine to ask Tachyon to publish this stuff
in their jars. Having Tachyon code inside of the Spark codebase is not good f
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/603#issuecomment-35328014
@nicklan @mateiz as an alternative, it might be simplest to just download a
Tachyon package (e.g. don't use maven to deal with this). And have a nice
gra
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/299#discussion_r9812541
To be consistent with the other naming this should be `camelCase` rather
than `lower-with-hyphens`.
Also, how about `spark.driver.addJars`. It seems to
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/299#discussion_r9812596
Now that I see this here, I wonder if maybe it just makes sense to note
this in the configuration doc below rater than here. It's really only relevant
to p
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/299#discussion_r9812694
I also think it's fine to ditch this and just include it in the config
docs... it's up to you though... I just realized after seeing this there is
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/299#discussion_r9812713
Makes sense, I think it's fine to just change the local:// semantics a bit.
---
If your project is set up for it, you can reply to this email and have your
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/299#discussion_r9812781
Mind formatting this to meet the style guide:
```
private[spark] val classLoader =
if (conf.getBoolean("spark.driver.add-dynamic-jars&qu
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/299#discussion_r9812824
I'm a bit confused on this one... if we set the context class loader to be
the new one in SparkContext, and akka's default is to get the context cla
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/299#discussion_r9812862
It would be great if it were possible to not modify the way akka
initializes... but there may be some reason I'm missing why we need to.
---
If your proje
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/299#issuecomment-35353411
@velvia took a pass on this. This will be a useful feature. One question
about the need to explicitly pass classloaders.
---
If your project is set up for it
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/585#discussion_r9834337
is it possible to exclude the entire `org.apache.spark.deploy` package?
---
If your project is set up for it, you can reply to this email and have your
reply
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/585#discussion_r9834543
I think you can do it with excludePackage:
https://github.com/typesafehub/migration-manager/blob/master/core/src/main/scala/com/typesafe/tools/mima/core
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/585#discussion_r9834629
Then you can add a comment that says "Scheduler is not considered a public
API"
---
If your project is set up for it, you can reply to this email and
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/585#discussion_r9834615
Let's just exclude the entire `ExternalAppendOnlyMap` class. The you can
add a comment that says "Was made private in 1.0"
---
If your project i
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/585#issuecomment-35416994
@ScrapCodes hm... I noticed a bigger issue that things which are package
private are not being filtered in MIMA and this is leading to a bunch of the
false
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/616#issuecomment-35457550
@aarondav this needs to be revered... it will break everywhere we tell
people how to depend on Spark:
scala-programming-guide.md:artifactId
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/616#issuecomment-35457734
I reverted this in master and 0.9
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/616#issuecomment-35457956
I'm looking into it with @aarondav right now... just want to make sure this
doesn't break anything.
---
If your project is set up for it, you ca
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/616#issuecomment-35459440
Hey so I looked at it more closely, we need to look on a case-by-case basis
where this is used. Some of the cases should be changed to 2.10.3 and others
should
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/617#issuecomment-35459569
Thanks this is a nice. LGTM.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/585#issuecomment-35471000
It turns out our issues are shortfallings of the existing MIMA tool. The
visibility of classes at the bytecode level is different than the visibility
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/incubator-spark/pull/618#discussion_r9870730
--- Diff: docs/index.md ---
@@ -19,7 +19,7 @@ Spark uses [Simple Build Tool](http://www.scala-sbt.org),
which is bundled with
sbt/sbt
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/618#issuecomment-35516252
LGTM pending a small fix -- @aarondav want to take a look?
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/618#issuecomment-35567163
Thanks guys I put this in master and 0.9.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/incubator-spark/pull/585#discussion_r9889925
--- Diff: project/MimaBuild.scala ---
@@ -0,0 +1,105 @@
+import com.typesafe.tools.mima.plugin.MimaKeys.{binaryIssueFilters,
previousArtifact
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/incubator-spark/pull/585#discussion_r9890048
--- Diff: project/MimaBuild.scala ---
@@ -0,0 +1,115 @@
+import com.typesafe.tools.mima.plugin.MimaKeys.{binaryIssueFilters,
previousArtifact
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/incubator-spark/pull/585#discussion_r9890019
--- Diff: project/MimaBuild.scala ---
@@ -0,0 +1,115 @@
+import com.typesafe.tools.mima.plugin.MimaKeys.{binaryIssueFilters,
previousArtifact
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/incubator-spark/pull/585#discussion_r9890236
--- Diff: project/MimaBuild.scala ---
@@ -0,0 +1,105 @@
+import com.typesafe.tools.mima.plugin.MimaKeys.{binaryIssueFilters,
previousArtifact
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/incubator-spark/pull/585#discussion_r9890431
--- Diff: project/MimaBuild.scala ---
@@ -0,0 +1,105 @@
+import com.typesafe.tools.mima.plugin.MimaKeys.{binaryIssueFilters,
previousArtifact
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/192#issuecomment-35580209
See SPARK-1110... I took down some notes there relevant to this:
https://spark-project.atlassian.net/browse/SPARK-1110
---
If your project is set up for it
1 - 100 of 169 matches
Mail list logo
