Improve command line handling of the execution app #1083
Conversation
|
|
||
| private[this] val hadoopReservedArgs = List("-fs", "-jt", "-files", "-libjars", "-archives") | ||
|
|
||
| def extractUserHadoopArgs(args: Array[String]): (Array[String], Array[String]) = { |
There was a problem hiding this comment.
tests for this function would be great. I think some scalacheck would be nice. Generate random Array[String] args and the non-hadoop args can't be parsable with the GenericOptionsParser.
|
ping. just wanted a minor refactoring to make the complex fold more readable. |
|
Sorry about the delay, been swamped. Should have the changes you requested now + some property based tests |
|
|
||
| property("Non-hadoop random args will all end up in the right bucket") = forAll { (args: Array[String]) => | ||
| val (hadoopArgs, nonHadoop) = ExecutionApp.extractUserHadoopArgs(args) | ||
| val res = hadoopArgs.toArray.isEmpty && nonHadoop.toArray.sameElements(args) |
There was a problem hiding this comment.
couldn't this fail if we randomly get a string that looks like "-Da=b"
There was a problem hiding this comment.
Yep, sort of just relying on that being improbable. I could filter out such
things in here ?
On Monday, November 10, 2014, P. Oscar Boykin notifications@github.com
wrote:
In
scalding-core/src/test/scala/com/twitter/scalding/ExecutionAppProperties.scala:+import org.scalacheck.Prop.forAll
+import org.scalacheck.Gen.choose
+import org.scalacheck.Prop._
+
+// Be careful here in that Array[String] equality isn't contents based. its java referenced based.
+object ExecutionAppProperties extends Properties("ExecutionApp Properties") {
- def debugPrint(inputArgs: Array[String], resultingHadoop: HadoopArgs, resultingNonHadoop: NonHadoopArgs): Unit = {
- val errorMsg = "Input Args: " + inputArgs.map(""" + _ + """).mkString(",") + "\n" +
"Hadoop Args: " + resultingHadoop.toArray.mkString(",") + "\n" +"Non-Hadoop Args: " + resultingNonHadoop.toArray.mkString(",") + "\n"- sys.error(errorMsg)
- }
- property("Non-hadoop random args will all end up in the right bucket") = forAll { (args: Array[String]) =>
- val (hadoopArgs, nonHadoop) = ExecutionApp.extractUserHadoopArgs(args)
- val res = hadoopArgs.toArray.isEmpty && nonHadoop.toArray.sameElements(args)
couldn't this fail if we randomly get a string that looks like "-Da=b"
—
Reply to this email directly or view it on GitHub
https://github.com/twitter/scalding/pull/1083/files#r20097201.
There was a problem hiding this comment.
It is super improbable, I guess.
Improve command line handling of the execution app
The execution app parser wasn't handling non -D hadoop args that the GenericOptionsParser needs correctly. This special cases all the special ones for the GOP