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

Solve intersection of CLI/signature default values vs config values #326

Open
bitprophet opened this issue Feb 8, 2016 · 2 comments
Open

Comments

@bitprophet
Copy link
Member

Ran into a corner case:

  • Task defined as def mytask(foo=False):
  • Inside the task, it allows a config path - say, myapp.foo - to override that default value of foo
  • Due to how we handle CLI args, there is thus only --foo (to flip it to True) and no --no-foo.
  • An environment where the config - e.g. conf file or collection-level - is setting myapp.foo = True, is thus unable to set foo to False at runtime via CLI flags.
  • The only way to work around it at present is to somehow present config values higher up than whatever's setting myapp.foo = True (e.g. environment variables, or a runtime config file w/ -f) which set myapp.foo = False.
  • That's all and well (and I'm glad I wrote this out cuz I plumb forgot those options exist - whee) but it's not terrifically intuitive - I myself kept trying to say --no-foo and wondering why it wasn't working.

There's also a more general case, which is simply:

  • The "usual" Python method of distinguishing runtime values from default values in a way that doesn't muddle None with False, is: kwarg=None, then inside a function, kwarg = kwarg if kwarg is not None else <some-default-from-somewhere>.
  • That doesn't work well here for Invoke tasks because of how much we derive from the signature, re: type. (though see CLI arg typecasting #9)
  • Which means that for any general "runtime flag == config key" setup, it's difficult to reliably consider the CLI flags to be another "config level" above all of the other config vectors. We simply can't tell if the user gave a flag or not, at function/task body execution time.
  • However, we can probably update Context/Config so it exposes parser data (I think we do this already or have a ticket for it) and then use that in a convenience method which yields the "actual" final value.
    • We could then update Executor to insert those values into the function call, leaving the function itself entirely free of this concern - it just deals with what it gets, and if the value "actually" came from config instead of CLI flag, that's not likely to be a concern.

The above could be its own ticket, but if implemented, it would also neatly solve the more corner-y case at top.

@hjoukl
Copy link

hjoukl commented Sep 15, 2021

Hi @bitprophet thanks for all your work on the fine invoke library! :-)

And sorry for the following lengthy post.

I've run into this case and banged my head against it for a while now. My use case: I want to provide a project configuration invoke.yaml alongside tasks.py and use some of its values as task parameter defaults (def mytask(ctx, myarg=<default value read from invoke.yaml>). To keep it DRY I essentially want every setting (defaulted) in invoke.yaml. Then, I'd like to be able to apply a runtime configuration e.g. -f my.invoke.yaml and use its settings if they override the defaults. Of course, I'd still like to be able to finally override these on the command line, e.g. --greet='foo bar'.

As you describe, inside the task function body you currently cannot distinguish if

  • the given arg is the default or
  • the given arg was provided by the user, on the command line

This basically only works for None default args, with the kwarg = kwarg if kwarg is not None else <some-default-from-somewhere> pattern, but then the nice boolean defaults handling stops working (i.e. no automatic --foo/--no-foo command line options, any more).

I've found a way to almost achieve what I want by

  • pre-reading the invoke.yaml project configuration, (ab)using the invoke loader machinery, to have the config values at task function definition time
  • using a custom Task class and
  • a "default wrapper" object,

like so:

import os.path

from invoke import Collection, Config, Task, task


# Our poor man's namespace for global names
class G:
    project_dir = os.path.dirname(__file__)

    # pre-read the project invoke.yaml to be able to use this for task default
    # parameters and as a template for runtime configuration files
    default_config = Config(project_location=project_dir)
    default_config.load_project()
    # a handier shortcut
    defaults = default_config.hello


# Helper class to tunnel Default instances through the invoke machinery.
class DistinguishableDefaultsTask(Task):
    def get_arguments(self):
        # Py2 compat syntax
        args = super(DistinguishableDefaultsTask, self).get_arguments()

        for argument in args:
            if isinstance(argument.default, Default):
                # Trick invoke machinery into working everything out using the
                # original type wrapped by a Default object
                argument.kind = argument.default.kind
        return args


class Default:
    """A wrapper object for task signature default arguments.

    Its intention is to be able to decide wether a task function has been
    invoked with the default argument or with a command line-provided argument.
    """
    def __init__(self, value=None, kind=None):
        self.value = value
        self.kind = type(value) if kind is None else kind

    def __call__(self):
        return self.value

    def __bool__(self):
        return bool(self.value)

    # Py2 compat
    __nonzero__ = __bool__


# Better name? I like the shortness but _() is often used for other purposes...
def _(arg_value, ctx_value):
    """Return ctx_value if arg_value is a 'Default' object, arg_value
    otherwise.
    """
    if isinstance(arg_value, Default):
        return ctx_value
    return arg_value


@task(
    auto_shortflags=False,
    klass=DistinguishableDefaultsTask,
    )
def hello(ctx,
          greet=Default(G.defaults.tasks.hello.opts.greet),
          greet_targets=Default(G.defaults.tasks.hello.opts.greet_targets),
          ):
    """Hello.
    """
    if ctx.config.run.echo:
        print(f'ctx={ctx}')
        print(f'greet={greet}')
        print(f'greet_targets={greet_targets}')
        print(f'ctx.hello.tasks.hello.opts.greet='
              f'{ctx.hello.tasks.hello.opts.greet}')
        print(f'ctx.hello.tasks.hello.opts.greet_targets='
              f'{ctx.hello.tasks.hello.opts.greet_targets}')

    # get values from ctx if defaults
    greet = _(greet, ctx.hello.tasks.hello.opts.greet)
    greet_targets = _(greet_targets, ctx.hello.tasks.hello.opts.greet_targets)

    if ctx.config.run.echo:
        print(f'*** applied: greet={greet}')
        print(f'*** applied: greet_targets={greet_targets}')

    if greet:
        ctx.run("echo 'Hello {}'".format(greet_targets))


ns = Collection(hello)

With that, we can now override properly:

$ cat invoke.yaml 
hello:
  tasks:
    hello:
      opts:
        greet: true
        greet_targets: project configuration file
$ cat my.invoke.yaml 
hello:
  tasks:
    hello:
      opts:
        greet_targets: runtime configuration file
0 $ invoke hello
Hello project configuration file
0 $ invoke hello -f my.invoke.yaml 
Hello runtime configuration file
0 $ invoke hello -f my.invoke.yaml --greet-targets='cmdline'
Hello cmdline
0 $ invoke hello --greet-targets='cmdline'
Hello cmdline

Apart from one little thing:

$ invoke hello --help
Usage: inv[oke] [--core-opts] hello [--options] [other tasks here ...]

Docstring:
  Hello.


Options:
  --greet
  --greet-targets=STRING

The intended --greet/--no-greet is not there due to this ParserContext code. The problem here is the arg.kind == bool and arg.default is True test - to make it work properly it needed to be relaxed to a simpler arg.kind == bool and arg.default test. In other words only test for "truthiness", not for identity with the True singleton:

$ git diff
diff --git a/invoke/parser/context.py b/invoke/parser/context.py
index 6826df64..331f1044 100644
--- a/invoke/parser/context.py
+++ b/invoke/parser/context.py
@@ -141,7 +141,7 @@ class ParserContext(object):
         if arg.attr_name:
             self.args.alias(arg.attr_name, to=main)
         # Add to inverse_flags if required
-        if arg.kind == bool and arg.default is True:
+        if arg.kind == bool and arg.default:
             # Invert the 'main' flag name here, which will be a dashed version
             # of the primary argument name if underscore-to-dash transformation
             # occurred.

(tests run successfully with this change)

I quite like the approach for my (limited) use case but can't judge if it could work on a broader scale. I do think however that a more general solution like changing the executor machinery to call the task function with some kind of "finalized" value (i.e. taken from the parameter default or from a config file (which path x.y.z of the config?) if given or from the command line if given is way more complex and probably an incompatible change.

Any chance of the above minimal change to the "bool-arg-inversion" logic getting accepted? I'd provide a PR then...

Of course, the original Task class could also grow capabilities to support such Default objects as DistinguishableDefaultsTask in my example, but that's for later. ;-)

Best regards,
Holger

@hjoukl
Copy link

hjoukl commented Sep 15, 2021

A simple PR says more than a thousand words :-): #812

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

No branches or pull requests

2 participants