Proposal Context object

[johnynek]

Right now, methods that read or write have to take an implicit FlowDef and Mode. I propose that we define a Context object like:

class Context {
  def flowDef: FlowDef
  def mode: Mode
  def args: Args
}

This allows us to extend features to include things like (tracing, see: https://github.com/etsy/scalding/pull/4) without having to rewrite existing methods.

Also, this fixes an old issue around getting the mode implicitly. Now, we statically set one mode in Mode.mode. This breaks thread-safety and prevent parallel testing. With this approach, the Context can be created in the Tool, and passed into the Job.

This is a pretty big change.

For most functions that don't explicitly pass implicit variables, changing from (implicit fd: FlowDef, mode: Mode) to (implicit ctx: Context) won't be an issue, but all jobs have a single Args parameter input. How to pass Context instead?

My work around, which is ugly, would be to make Context a subclass of Args. Then in job, add a method:

implicit def context: Context = {
  args match {
    case ctx: Context => ctx
    case _ => sys.error("Given an Args that is not an instance of Context")
  }
}

The "right" way to go would be to just change Job to accept Context. If we do the right way, we should probably write a script of some kind that can automatically convert source code to the new format with a minimum of pain.

I invite a design discussion here.

[rjhall]

I think you could have Job still take only the Args in the constructor, then immediately set up the context itself, rather than setting up the context in the runner and passing it to the Job. This way backwards compatibility would be much nicer.

It would require somehow stuffing the jobconf into the args though, so that the job would be able to set up the mode.

[johnynek]

Great suggestion:

https://github.com/twitter/scalding/blob/develop/src/main/scala/com/twitter/scalding/Tool.scala#L68

We could just do that logic in a method:

implicit def mode: Mode = Mode(args, getConf(args))

The issue, is we need access to the jobConf. Any suggestions there?

[rjhall]

It could be passed implicitly (although the whole point of this idea is to cut down on implicits). Maybe for the jobconf this isn't so bad.

[johnynek]

How could we do that? All the job is wired in the constructor of the Job. We have to have the getConf in place when the constructor is called.

This is the same issue we dealt with in Mode by just putting it in the static implicit mode parameter in the Mode companion object. That solution breaks parallelism in the unit tests (and dramatically slows them down as a result).

[avibryant]

This isn't fully thought out, but - JobConf and Args are both effectively Map[String,String], and they're very unlikely to have conflicting namespaces. It doesn't seem unreasonable to me to unify them (so the runner would dump all of the JobConf properties into Args, and then the Job would construct a new JobConf from Args).

[rjhall]

Args could even just store the jobconf as a field with a get method.

[johnynek]

Avi, I like your approach. The only subtlety is that we actually serialize the args into the jobConf for debugging, so we'd have to pull it out there, or we'd create a lot of waste.

Anyone want to take a stab at a pull request in the 0.9.0 branch?