Skip to content
This repository has been archived by the owner on Jan 9, 2020. It is now read-only.

Disable ssl test for staging server if current classpath contains the jetty shaded classes #573

Open
wants to merge 6 commits into
base: branch-2.2-kubernetes
Choose a base branch
from

Conversation

echarles
Copy link
Member

What changes were proposed in this pull request?

The ResourceStagingServerSuite.Enable SSL on the server will will run only if the current classpath does not contain the shaded jetty classes.

If you rely on the spark-core.jar in your classpath, this test will not run.
If you rely on the spark-core module (without shading), this test will be run.

This is to address issues discussed in #463.

To allow test with spark-core.jar, we also add the jetty deps in the kubernetes/core pom (see also #570).

How was this patch tested?

Run maven test phase from the local spark/core module or from the top of the spark maven hierarchy.

}

try {
Class.forName("org.spark_project.jetty.util.ssl.SslContextFactory")
Copy link

Choose a reason for hiding this comment

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

use Utils.classForName, it's the reason why Jenkins is failing the build here.

@echarles
Copy link
Member Author

@mccheah I have pushed an update using Utils.classForName instead of Class.forName to avoid scalacheckstyle error. Let's see what Jenkins tells us.

@echarles
Copy link
Member Author

@mccheah build is Successfull, but showed as Failed due to a Jenkins post-build action to update the Github PR

Could not update commit status of the Pull Request on GitHub.
...
Caused by: com.fasterxml.jackson.core.JsonParseException: Numeric value (4480632093) out of range of int
 at [Source: java.io.StringReader@2b9cc297; line: 1, column: 130]

@ssuchter
Copy link
Member

ssuchter commented Feb 5, 2018

retest this please

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants