Skip to content

Conversation

@vanzin
Copy link
Contributor

@vanzin vanzin commented Aug 6, 2015

RUtils.isRInstalled throws an exception if R is not installed,
instead of returning false. Fix that.

RUtils.isRInstalled throws an exception if R is not installed,
instead of returning false. Fix that.
@SparkQA
Copy link

SparkQA commented Aug 7, 2015

Test build #40084 has finished for PR 8008 at commit df72d8c.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@vanzin
Copy link
Contributor Author

vanzin commented Aug 7, 2015

retest this please

@SparkQA
Copy link

SparkQA commented Aug 7, 2015

Test build #40134 has finished for PR 8008 at commit df72d8c.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@shivaram
Copy link
Contributor

shivaram commented Aug 7, 2015

Jenkins, retest this please

@shivaram
Copy link
Contributor

shivaram commented Aug 7, 2015

LGTM. Thanks @vanzin for the fix. cc @brkyvz

@brkyvz
Copy link
Contributor

brkyvz commented Aug 7, 2015

LGTM. Thanks @vanzin

@SparkQA
Copy link

SparkQA commented Aug 7, 2015

Test build #40173 has finished for PR 8008 at commit df72d8c.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@shivaram
Copy link
Contributor

shivaram commented Aug 7, 2015

Sigh -- our tests are too flaky.

Jenkins, retest this please

@shivaram
Copy link
Contributor

shivaram commented Aug 7, 2015

Jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Aug 8, 2015

Test build #40204 has finished for PR 8008 at commit df72d8c.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@shivaram
Copy link
Contributor

shivaram commented Aug 8, 2015

Sigh, I guess today is not a good day to get tests passed.

@shivaram
Copy link
Contributor

shivaram commented Aug 8, 2015

Jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Aug 8, 2015

Test build #40242 has finished for PR 8008 at commit df72d8c.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@shivaram
Copy link
Contributor

shivaram commented Aug 8, 2015

Jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Aug 9, 2015

Test build #40250 has finished for PR 8008 at commit df72d8c.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@shivaram
Copy link
Contributor

shivaram commented Aug 9, 2015

@JoshRosen @rxin This PR has failed Jenkins 6 (!) times with unrelated failures. Are there any known issues or flaky tests ? I'd like to get this fix into 1.5, but can't merge this right now

@rxin
Copy link
Contributor

rxin commented Aug 9, 2015

I started 4 different Jenkins run for this just now.

@SparkQA
Copy link

SparkQA commented Aug 9, 2015

Test build #1415 has finished for PR 8008 at commit df72d8c.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@shivaram
Copy link
Contributor

shivaram commented Aug 9, 2015

I give up. I'm going to manually patch this on a clean branch on my laptop and run dev/run-tests and report back if it passes

@SparkQA
Copy link

SparkQA commented Aug 9, 2015

Test build #1418 timed out for PR 8008 at commit df72d8c after a configured wait of 175m.

@shivaram
Copy link
Contributor

shivaram commented Aug 9, 2015

Jenkins, retest this please

@shivaram
Copy link
Contributor

shivaram commented Aug 9, 2015

@tdas @rxin I've see the test SPARK-6222: Do not clear received block data too soon fail a bunch of times (on Jenkins and on local runs) - Example stack trace at [1]. There is already an open JIRA saying this is a flaky test at https://issues.apache.org/jira/browse/SPARK-7420 -- Can we disable the test for now as its becoming really hard to merge PRs ?

[1] https://gist.github.com/shivaram/84a2b47ada5509c707fd

@rxin
Copy link
Contributor

rxin commented Aug 9, 2015

I disabled the test.

@SparkQA
Copy link

SparkQA commented Aug 9, 2015

Test build #40272 has finished for PR 8008 at commit df72d8c.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@shivaram
Copy link
Contributor

shivaram commented Aug 9, 2015

Jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Aug 10, 2015

Test build #40278 has finished for PR 8008 at commit df72d8c.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@shivaram
Copy link
Contributor

Jenkins, retest this please

@AmplabJenkins
Copy link

Merged build finished. Test FAILed.

@vanzin
Copy link
Contributor Author

vanzin commented Aug 10, 2015

So, we really need to start disabling these flaky tests and filing bugs, because waiting for jenkins to pass a build lately has been such a huge waste of time.

On the other hand, the actual tests that use this code have all passed in all builds, so can we just push this change?

@shivaram
Copy link
Contributor

Yeah I was wondering the same thing. In fact I think the tests have managed to pass piece-wise (i.e. core sometimes, streaming sometimes, sql sometimes etc.) that if you aggregate all of them everything has passed at this point.

The function isRInstalled is only used in RPackageUtilsSuite and SparkSubmitSuite, both of which are in core and have passed tests multiple times. I'm going to merge this change because I'm tired of waiting for jenkins, but adding up build scores across builds might be an idea for the future (cc @JoshRosen ?)

@rxin
Copy link
Contributor

rxin commented Aug 10, 2015

Let's merge them.

On Monday, August 10, 2015, Shivaram Venkataraman [email protected]
wrote:

Yeah I was wondering the same thing. In fact I think the tests have
managed to pass piece-wise (i.e. core sometimes, streaming sometimes, sql
sometimes etc.) that if you aggregate all of them everything has passed at
this point.

The function isRInstalled is only used in RPackageUtilsSuite and
SparkSubmitSuite, both of which are in core and have passed tests
multiple times. I'm going to merge this change because I'm tired of waiting
for jenkins, but adding up build scores across builds might be an idea for
the future (cc @JoshRosen https://github.com/JoshRosen ?)


Reply to this email directly or view it on GitHub
#8008 (comment).

@asfgit asfgit closed this in 0f3366a Aug 10, 2015
@vanzin vanzin deleted the SPARK-9710 branch August 11, 2015 23:37
CodingCat pushed a commit to CodingCat/spark that referenced this pull request Aug 17, 2015
RUtils.isRInstalled throws an exception if R is not installed,
instead of returning false. Fix that.

Author: Marcelo Vanzin <[email protected]>

Closes apache#8008 from vanzin/SPARK-9710 and squashes the following commits:

df72d8c [Marcelo Vanzin] [SPARK-9710] [test] Fix RPackageUtilsSuite when R is not available.
@robbinspg
Copy link
Member

My 1.5 branch build is failing with as described in SPARK-9710 and I notice that this merge didn't make it into that branch. Any chance this will be backported?

asfgit pushed a commit that referenced this pull request Sep 23, 2015
RUtils.isRInstalled throws an exception if R is not installed,
instead of returning false. Fix that.

Author: Marcelo Vanzin <[email protected]>

Closes #8008 from vanzin/SPARK-9710 and squashes the following commits:

df72d8c [Marcelo Vanzin] [SPARK-9710] [test] Fix RPackageUtilsSuite when R is not available.

(cherry picked from commit 0f3366a)
Signed-off-by: Shivaram Venkataraman <[email protected]>
ashangit pushed a commit to ashangit/spark that referenced this pull request Oct 19, 2016
RUtils.isRInstalled throws an exception if R is not installed,
instead of returning false. Fix that.

Author: Marcelo Vanzin <[email protected]>

Closes apache#8008 from vanzin/SPARK-9710 and squashes the following commits:

df72d8c [Marcelo Vanzin] [SPARK-9710] [test] Fix RPackageUtilsSuite when R is not available.

(cherry picked from commit 0f3366a)
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.

7 participants