Skip to content
Closed
Changes from 5 commits
Commits
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
9 changes: 9 additions & 0 deletions core/src/main/scala/org/apache/spark/SparkConf.scala
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,15 @@ class SparkConf(loadDefaults: Boolean) extends Cloneable with Logging {
}
}
}

Copy link
Member

Choose a reason for hiding this comment

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

I can probably remove this on merge, but there's a stray blank here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I saw, usually there is a blank line before ending of long methods and classes. You might remove it if you will.

// System property spark.yarn.app.id must be set if user code ran by AM on a YARN cluster
// yarn-standalone is deprecated, but still supported
if ((get("spark.master") == "yarn-cluster" || get("spark.master") == "yarn-standalone") &&
Copy link
Contributor

Choose a reason for hiding this comment

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

if ((master == "yarn-cluster" || master == "yarn-standalone") && !_conf.contains("spark.yarn.app.id""))

I don't see any method named get so I'm not sure your code would even compile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are two gets. Tested on local and remote YARN clusters.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh my bad. I thought this was still in SparkContext. In fact, I think the check should be there. When I mentioned "check for spark.yarn.app.id in SparkConf" I didn't mean to change SparkConf, I meant to do as I suggested above (!_conf.contains("spark.yarn.app.id"")).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What we do here is actually an act of validating the configuration. It seems to be a good place inside validateSettings. There is no information required from outside to do this check.

Copy link
Contributor

Choose a reason for hiding this comment

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

SparkConf is not just for SparkContext.

get("spark.yarn.app.id", null) == null) {
throw new SparkException("Detected yarn-cluster mode, but isn't running on a cluster. " +
"Deployment to YARN is not supported directly by SparkContext. Please use spark-submit.")
}

}

/**
Expand Down