Pre-tasks and configuration do not mesh well #116

Closed
bitprophet opened this Issue Feb 27, 2014 · 4 comments

Projects

None yet

1 participant

@bitprophet
Member

Scenario:

  • Using a configured task/tree such as invocation.docs
  • Some other 'vanilla' task in the tree specifies one of those configured tasks in its pre-tasks list (e.g. @task('docs'))
  • Expected: the pre-task is able to find its configuration parameters, just as if it had been called by itself on the CLI
  • Instead: the pre-task is unable to find any config parameters.

Digging:

  • Config lookup is done based on the "name" of the task being executed
  • Right now that is always the actual task specified on the CLI; that may not be entirely accurate and should probably change to be the actual task being called (this is within the loop across "all tasks", aka post-expansion of pre/post tasks.
  • Further confusing things is that in my specific use case, I am calling a 'default' task and its name gets expanded at CLI parse time (probably due to use of aliases/Lexicons) when it's called directly, but this does not occur when it's used as a pre-task.
    • So e.g. $ inv docs has a "name" at execution time of docs.build, but when used as a pre-task it comes out as just build
  • Config lookup uses the "directory" or "package" level of a task name, so e.g. in the direct case, we see configuration('docs') called, but in the indirect case, it's configuration('').
  • I haven't gone past this yet but suspect this is the key, the config options are likely stored on the docs namespace and so any configuration() call not using docs won't load them.
  • So perhaps we need to do more seeking back up the object ref chain so a task knows where it is currently living/stored & thus which config to load?
    • Needs to not break existing config overriding patterns...
    • Feels like another way of saying "parser seeing docs and turning up docs.build needs to match the pre-task machinery seeing docs and ensure it also turns up docs.build"?
@bitprophet bitprophet referenced this issue in paramiko/paramiko Feb 27, 2014
Closed

Epydoc -> Sphinx. #256

@bitprophet
Member
  • Core issue is that tasks don't know where they live, and honestly should not; that responsibility lies in Collection.
  • In Executor, the pre-tasks come out of task_list
  • task_list is what asks the core/root Collection object for the pretask, via simple getitem/index semantics
  • The resulting list is what's iterated over when running pre-tasks
  • So it feels like we need to do "more" with that Collection object such that we arrive at the right config for each pre-task
    • I think what this means is that configuration() needs to not take a path, but instead the task itself? Perhaps?
    • Or, again, we normalize the name used to refer to the task - so in this case, the string name in pre (aka the 'docs' in @task('docs')) must turn into the "canonical" name for the task, which would be docs.build.
      • Which, in the direct use case, is the key in the Lexicon handed to the parser machinery - e.g. {'docs.build': ['docs']}
      • And it is docs.build because the request for to_contexts is coming from the root collection (which would not be changing in any of this)
      • And to_contexts just calls task_names which is what builds the actual mapping.
    • Thus what we need is an analogue to Collection[taskname] which yields something like ('real.canonical.name', <Task>)
    • But this brings us back to "the Collection is what knows that when it is asked for task 'foo', that task's canonical name is 'module.foo' and its config should reflect this"
    • So I should check for existing uses of Collection.configuration and probably change it to take a task name instead of a path, thus pushing all of this sort of calculation within Collection and then client code doesn't need to care.
@bitprophet bitprophet added a commit that referenced this issue Feb 28, 2014
@bitprophet bitprophet Failing test proving #116 5e64d43
@bitprophet
Member

As I work it feels more and more like we really need collection[name] to yield a Task that knows its canonical name. We start Executor.execute with just the name, but must hop back and forth between name and task object, both for it and its pretasks, for purposes of both deduping/ensuring no double-calling, and also for the configuration stuff.

It probably can be done without doing this - just pass around both bits of info as needed - just feels maybe a bit verbose. Will see.

@bitprophet
Member

Dumb issue with that idea, the tests use an 'idealized' handful of Collection objects with no tasks. Works fine for the "ask for the collection-level name/path" approach; but not when we switch it to be task focused. Still, feels dumb to try and work around it just for shorter tests. Will take a shot.

@bitprophet
Member

Herpaderp, all tests pass now, real world use case works now, YAY.

@bitprophet bitprophet closed this in 5cfb85f Mar 4, 2014
@bitprophet bitprophet added a commit that referenced this issue Mar 4, 2014
@bitprophet bitprophet Forgot a changelog re #116 a7ca435
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment