Skip to content
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.

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

@task_param() decorator to pass command line parameters to task generators and @create_after for class defined tasks #323

Closed
wants to merge 12 commits into from

Conversation

rbdixon
Copy link
Contributor

@rbdixon rbdixon commented Jul 28, 2019

As described in #311 this decorator can be used to pass command line arguments to the task generator. The same parameter definitions are also passed to the generated tasks.

This branch also includes pull request #307 which implements @create_after() semantics for tasks defined in a class. As discussed on #307 @schettino72 asked to see code for #311 as well. If this PR is merged then #307 will be closed.

Quick implementation to see if this is what was intended by the issue and to open discussion.

Work to be done:

  • actually do the parsing!
  • auto compatibility
  • docs since this is a new user-facing feature
  • tests
    • Ensure that a task parameter can be passed to the task generator.
    • Ensure that a task generator parameter can be set from the command line.
    • Ensure that a task parameter can be passed to the task generator defined as a class method.

Demo:

$ cat dodo.py
from doit import create_after, task_param


def task_early():
    return {"actions": ["echo early"], "verbosity": 2}


@create_after(executed="early")
@task_param([{"name": "foo", "default": "bar", "long": "foo"}])
def task_use_param_create_after(foo):
    print(f'Task parameter foo={foo} available at task definition time.')

    def runit():
        print(f"param foo={foo}")

    return {"actions": [runit], "verbosity": 2}


@task_param([{"name": "foo", "default": "bar", "long": "foo"}])
def task_use_param(foo):
    print(f'Task parameter foo={foo} available at task definition time.')

    def runit():
        print(f"param foo={foo}")

    return {"actions": [runit], "verbosity": 2}


@task_param([{"name": "howmany", "default": 3, "type": int, "long": "howmany"}])
def task_subtasks(howmany):
    for i in range(howmany):
        yield {"name": i, "actions": [f"echo I can count to {howmany}: {i}"]}

def do_work(foo):
    print(f'Argument foo={foo}')

@task_param([{"name": "foo", "default": "bar", "long": "foo"}])
def task_use_in_action(foo):
    print(f'When the task action runs it will print {foo}')

    return {
        'actions': [do_work],
        'verbosity': 2
    }

@task_param([{'name': 'num_tasks', 'default': 1, 'type': int, 'long': 'num_tasks'}])
def task_subtask(num_tasks):
    print(f'Generating {num_tasks} subtasks')

    def work(task_num, num_tasks):
        print(f'Task {task_num+1} of {num_tasks}')

    for i in range(0, num_tasks):
        yield {
            'name': f'task{i}',
            'actions': [(work, (), {'task_num': i})],
            'verbosity': 2
        }

$ cat doit.cfg
[task:use_param_create_after]
    foo = from_doit_cfg
$ doit
Task parameter foo=bar available at task definition time.
When the task action runs it will print bar
Generating 1 subtasks
.  early
early
Task parameter foo=bar available at task definition time.
.  use_param_create_after
param foo=bar
.  use_param
param foo=bar
.  subtasks:0
.  subtasks:1
.  subtasks:2
.  use_in_action
Argument foo=bar
.  subtask:task0
Task 1 of 1

$ doit info use_param
Task parameter foo=bar available at task definition time.
Task parameter foo=bar available at task definition time.
When the task action runs it will print bar
Generating 1 subtasks

use_param

status     : run
 * The task has no dependencies.

params     : 
 - {'name': 'foo', 'default': 'bar', 'long': 'foo'}

verbosity  : 2

$ doit info use_param_create_after
Task parameter foo=bar available at task definition time.
Task parameter foo=bar available at task definition time.
When the task action runs it will print bar
Generating 1 subtasks

use_param_create_after

status     : run
 * The task has no dependencies.

params     : 
 - {'name': 'foo', 'default': 'bar', 'long': 'foo'}

verbosity  : 2

@rbdixon
Copy link
Contributor Author

rbdixon commented Jul 28, 2019

Task parameters defined with @task_param() are passed down to the task and any subtasks right now. This could be a feature or a bug? What do you think?

8/13: Absent any input I decided this is a feature.

@rbdixon
Copy link
Contributor Author

rbdixon commented Jul 28, 2019

hmm... well, I've got a bug buried down there coping with @create_after tasks. I'll fix that soon.

@rbdixon
Copy link
Contributor Author

rbdixon commented Aug 13, 2019

I believe this is ready to merge. I'm using this locally with my own doit-based automations and nikola. So far, so good.

@rbdixon rbdixon marked this pull request as ready for review August 13, 2019 16:34
@rbdixon
Copy link
Contributor Author

rbdixon commented Aug 22, 2019

Hmmm.... I've found a wrinkle and I think I'd like input on what the best behavior would be:

Given dodo.py:

from doit import task_param

DRYRUN = {
    'name': 'dry_run',
    'short': 'n',
    'long': 'dry-run',
    'type': bool,
    'default': False,
    'help': 'Do not commit database changes.',
}


@task_param([DRYRUN])
def task_test(dry_run):
    def _subtask(i, dry_run):
        print(f'subtask {i}, dry_run=={dry_run}')

    print(f'Task creation, dry_run == {dry_run}')
    yield {'name': None, 'verbosity': 2}

    for i in range(1):
        yield {
            'name': f'subtask_{i}',
            'actions': [(_subtask, (i,), {})],
            'verbosity': 2,
        }

Run:

$ doit test -n
Task creation, dry_run == True
.  test:subtask_0
subtask 0, dry_run==False

The parameter definition was propagated to the subtask but not the value. The task dry_run value can be pushed to the subtask in code, of course.

Two options:

  1. Keep the current behavior. The @task_param definitions propagate to subtasks but not the runtime value.
  2. Propagate the runtime value of the param to subtasks.

Option 2 seems "least surprising". Opinions?

@rbdixon
Copy link
Contributor Author

rbdixon commented Oct 4, 2019

@schettino72 ^ just checking in to see if you had comments on the above. Thanks.

@schettino72
Copy link
Member

@rbdixon sorry for very long delay. I should be able to merge this and release and next few weeks.

@schettino72
Copy link
Member

Task parameters defined with @task_param() are passed down to the task and any subtasks right now. This could be a feature or a bug? What do you think?

I dont think it desirable to pass to subtasks. And since params is a simple dict it would be trivial to explicitly pass it into subtasks.

But I think it should be part of base task. So it is documented by help command.
Of course things get a bit complicated if base are explicitly defined. For those I would just not bother unless explicitly set.

@rbdixon
Copy link
Contributor Author

rbdixon commented Dec 20, 2019

Thanks for the comment. I'll work on this in January to:

  • Prevent passing @task_param() definitions down to subtasks.
  • Look at how to get @task_param() definitions included in the task help output.

@schettino72
Copy link
Member

@rbdixon could youp please rebase the PR to latest master?
some commits already merged. its a bit messy...

The @create_after decorator stashes a reference to the unbound method
in a DelayedLoader instance. To work with tasks defined by methods the
reference must be bound to the task creating class instance.
Add @task_param decorator to define parameters that will be available
to the task generator and the tasks as well.
@rbdixon
Copy link
Contributor Author

rbdixon commented Feb 7, 2020

New branch rebased to master implementing only the #311. Still need to implement below as discussed in December:

  • Prevent passing @task_param() definitions down to subtasks.
  • Look at how to get @task_param() definitions included in the task help output.

@rbdixon
Copy link
Contributor Author

rbdixon commented Feb 7, 2020

  • Subtasks now no longer "inherit" @task_param() defined command line arguments from the parent task.
  • There is a check which confirms that there are no duplicated names in the task parameter definitions. Just a useful check.
  • @task_param() defined parameters are include in info command output:
$ doit info subtask
Task parameter foo=bar available at task definition time.
Task parameter foo=bar available at task definition time.
When the task action runs it will print bar
Generating 1 subtasks

subtask

status     : run
 * The task has no dependencies.

task_dep   : 
 - subtask:task0

params     : 
 - {'name': 'num_tasks', 'default': 1, 'type': <class 'int'>, 'long': 'num_tasks'}
  • The CI build failures are due to other factors present in master.

I believe this final version satisfies all requests and comments and is ready to merge to master.

@rbdixon
Copy link
Contributor Author

rbdixon commented Feb 11, 2020

@schettino72 Any further work required for this PR?

@schettino72
Copy link
Member

sorry, i will have time only in march (hopefully)

@rbdixon
Copy link
Contributor Author

rbdixon commented Feb 12, 2020

That’s OK... I understand. I appreciate your dedication to doit!

@rbdixon
Copy link
Contributor Author

rbdixon commented Jun 26, 2020

Bump.... would love to see this merged. Thanks.

@schettino72 schettino72 added this to the 0.34 milestone Nov 19, 2021
@schettino72
Copy link
Member

I have merged this. I did some clean-up and fixes but still not complete.

  • Getting the command line arguments is implemented in very naive way and works only for the first task.
  • Using Task params on dict data together with task_param does not work as there is no way to parse when you have not read task dict.
  • I will rename task_param to task_params

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants