Skip to content

Fix SPARK-1629: Spark should inline use of commons-lang `SystemUtils.IS_... #569

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

Closed
witgo wants to merge 3 commits into from
Closed

Fix SPARK-1629: Spark should inline use of commons-lang `SystemUtils.IS_... #569

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
49e248e
Fix SPARK-1629: Spark should inline use of commons-lang `SystemUtils.…
witgo Apr 27, 2014
fcaafd7
merge mastet
witgo Apr 27, 2014
31520eb
Merge branch 'master' of https://github.com/apache/spark into SPARK-1629
witgo Apr 30, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
merge mastet
  • Loading branch information
@witgo
witgo committed Apr 27, 2014
commit fcaafd7d14153c2daefd8276d6e437db166d6427
6 changes: 6 additions & 0 deletions core/src/main/scala/org/apache/spark/util/Utils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1062,4 +1062,10 @@ private[spark] object Utils extends Logging {
def isWindows = Option(System.getProperty("os.name")).
Copy link
Contributor Author

@witgo witgo Apr 27, 2014

Choose a reason for hiding this comment

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

@srowen

def isWindows(): Boolean = {
  try {
    val osName = System.getProperty("os.name")
    osName != null && osName.startsWith("Windows")
  } catch {
    case e: SecurityException => (log a warning and return false)
  }
}

You think here will be thrown a SecurityException .Why?

Copy link
Member

@srowen srowen Apr 27, 2014

Choose a reason for hiding this comment

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

Regarding SecurityException, System.getProperty will often be disallowed in an environment where the security manager has been enabled. I strongly suspect that the rest of Spark fails already in such an environment for other reasons, and you could argue that the only reasonable response here is to fail rather than "guess" that the environment is not Windows. But I put it in mostly to mimic SystemUtils. It could go either way.

map(_.startsWith("Windows")).getOrElse(false)

/**
* Indicates whether Spark is currently running unit tests.
*/
private[spark] def isTesting = {
sys.env.contains("SPARK_TESTING") || sys.props.contains("spark.testing")
}
}
You are viewing a condensed version of this merge commit. You can view the full changes here.