pydoit / doit

CLI task management & automation tool
http://pydoit.org
MIT License
1.87k stars 175 forks source link

Specifying options (params) for subtasks #321

Closed smarie closed 5 years ago

smarie commented 5 years ago

A step towards #311 .

I managed to make my propoal work: i.e. a return statement after the generator in a group task will modify the params on the group task AND the ones in the subtasks.

So you can for example write:

from doit.tools import title_with_actions

def task_mytask():
    yield {
            'name': 'mysubtask',
            'actions': ["echo myparam is %(myparam)s"],
            'title': title_with_actions,
            'verbosity': 2,
        }

    # new: the final return statement defines params
    # for the group task AND all yielded subtasks
    return {'params': [{'name': 'myparam',
                        'long': 'myparam',
                        'default': None}]}

Currently it only works if the parameter is passed to a named subtask through the CLI: for example doit mytask:mysubtask --myparam hello will give

.  mytask:mysubtask => Cmd: echo myparam is %(myparam)s
myparam is hello

(By the way the title is not updated, this is maybe another issue.)

However the parameter is not correctly propagated if I call the group task instead doit mytask --myparam hello yields

.  mytask:mysubtask => Cmd: echo myparam is %(myparam)s
myparam is None

Any idea why ?

I can not figure out from the documentation how to propagate a parameter from the CLI or from the configuration file to a task without naming the task (and in this case the subtask) explicitly. Any help would be appreciated so that I can finalize the proposal.

The PR still misses unit tests and conceptual validation of what is been done: as of now I copy the group task params to all subtasks.

Thanks!

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.02%) to 99.717% when pulling 695d96940ddfab098eba8a465295213199ff18af on smarie:fix_issue_311 into f1037843eecd8d2078bcd84bd850488c3cecc22e on pydoit:master.

smarie commented 5 years ago

I added positive and negative tests. The problem with Appveyor does not seem related to my commits (database teardown issue).

schettino72 commented 5 years ago

This design does not solve the case where argument values are used in the task-creator itself. And I think the interface is completely non-intuitive. So a no go, sorry.

smarie commented 5 years ago

This design does not solve the case where argument values are used in the task-creator itself.

So do you mean what letsdoit plugin is already doing ? Then ok, that's another topic indeed.

But still I can not determine from the documentation (and I read it quite a number of times now) why the parameters

I'll try to reverse-engineer the code on monday to understand why and if it is a bug or a misunderstanding from my part.

schettino72 commented 5 years ago

This design does not solve the case where argument values are used in the task-creator itself.

So do you mean what letsdoit plugin is already doing ?

I mean to be able to use argument value while creating the task dict. Not only on task execution (actions). Something like this

from doit import task_param

@task_param({'name':'mytarget', 'short':'t', 'default':'result.txt'})
def task_py_params(mytarget):
        return {
            'name': name,
            'actions':['echo hello > %(targets)'],
            'targets': [mytarget]
        }
schettino72 commented 5 years ago
can not flow from the doit.cfg config file to the task (I can put the myparam parameter in any section, it is not propagated)

yes. this is a limitation: see #283

can not flow from the commandline to the subtask when the subtask is not explicitly named on the command:

There is no such a thing of flow of parameters, but...

    doit --myparam hello does not propagate it

You can use get_var to pass global arguments from command line.

https://pydoit.org/task_args.html?highlight=get_var#command-line-variables-doit-get-var

    doit mytask --myparam hello does not propagate it

This should be achievable after #311 is implemented

rbdixon commented 5 years ago

See PR #322 for an implementation for issue #283 to allow task parameters to be defined in the config file.

smarie commented 5 years ago

@schettino72 I just read your message again, and it seems that therefore one feature is missing and not already present in the backlog: the capability to expose task parameters to the commandline even when several tasks are run (typically when the default task sequence is run, and the user only calls doit).

Even if getargs can be used for this, from a user perspective it is very difficult to understand why it is there because it seems that task parameters are already covering that aspect. Except that they are not, so users need to "reconciliate" both, which is not handy.

Shall we open a ticket for this ? I have to admit that you have been closing tickets so fast in the past days that now I hesitate a bit ;)

schettino72 commented 5 years ago

@smarie have you read this? https://github.com/pydoit/doit/blob/master/.github/ISSUE_TEMPLATE/feature_request.md

If you follow your tickets will have a better chance of not being closed "so fast".

smarie commented 5 years ago

I have, actually :) and I tried my best to follow it. Apparently that was not enough 😄 Let me try once again then, if the above is not yet in the backlog or doc.

rbdixon commented 5 years ago

Very short comment since this is closed... IF @schettino72 thought it was a good idea to have a group-level means to define subtask params THEN I'd suggest moving this from being return-based to using yield with name: None as shown in https://pydoit.org/tasks.html#avoiding-empty-sub-tasks. To me, though, the benefit of this feature is not enough to be included since it is simple enough to use Python to accomplish the same objective.

smarie commented 5 years ago

Thanks @rbdixon for this suggestion !

Indeed this is interesting: if there is an existing mechanism to set common docand watch attributes, then it could be used to set common parameters. In that case there is no need to rely on the return statement.

It is strange that I did not come across this mechanism in the code where the subtasks are generated... Maybe this is done in a later step.