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

[WIP] Add automatic CLI generation from GW tasks #314

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

kotfic
Copy link
Contributor

@kotfic kotfic commented Oct 1, 2018

This PR relies on girder/girder_worker_utils#30 and provides a proof of concept implementation that relies on a terrible argument metadata API for auto-generation of a girder worker CLI. Consider the following girder worker task:

import click
from girder_worker.app import app
from girder_worker_utils.decorators import parameter


@parameter('a', cli_opts={'type': click.INT})
@parameter('b', cli_opts={'type': click.INT})
@parameter('zero',
           cli_args=('-z', '--zero'),
           cli_opts={'type': click.BOOL,
                     'help': 'return "undefined" if dividing by zero'})
@app.task(bind=True)
def divide(self, a, b, zero=False):
    if zero and b == 0:
        return 'undefined'

    return a / b

This implements a gwrun command that automatically discovers installed tasks and provides a click based application. E.g.:

⇒ gwrun --help
Usage: gwrun [OPTIONS] COMMAND [ARGS]...

Options:
  -o, --output [json|stdout]
  --help                      Show this message and exit.

Commands:
  divide    Proxy that evaluates object once.

And

 ⇒ gwrun divide --help
Usage: gwrun divide [OPTIONS] A B

  Proxy that evaluates object once.

  :class:`Proxy` will evaluate the object each time, while the promise will
  only evaluate it once.

Options:
  -z, --zero BOOLEAN  return "undefined" if dividing by zero
  --help              Show this message and exit.

Finally

⇒ gwrun divide 2 2     
1.0

The cli_args and cli_opts implementation (which is obviously ugly and confusing) are designed to be the low-level API on which a more intuitive and "intelligent" API may be built. For instance:

@parameter('a', type=float)
@parameter('b', type=float)
@app.task
def add(a, b):
    return a + b

represents a much nicer API and a mapping could be used to negotiate the difference between type=float and cli_opts={'type': click.FLOAT} behind the scenes. This serves the purpose of:

  1. Improving the readability of the API for the majority usecase
  2. Supporting a similar abstraction for handling automatic generation of a web UI

Note that this also attempts to implement basic output handling whereby the return value of the function may be directly printed to stdout (e.g. __repr__) or serialized to json before being printed.

@kotfic
Copy link
Contributor Author

kotfic commented Oct 1, 2018

To test this I recommend the following:

mkdir gwruntest; cd gwruntest

# Make a virtualenv
mkvirtualenv -a . gwruntest

# Clone the repos
git clone -b gwrun  git@github.com:girder/girder_worker.git
git clone -b gwrun_argspec git@github.com:girder/girder_worker_utils.git
git clone https://github.com/kotfic/gw_math_tasks.git

# Install the repos
pip install -e girder_worker_utils
pip install -e girder_worker
pip install -e gw_math_tasks

# Test the command
gwrun add 2 2
gwrun --help

Note there is an implementation of a toy plugin at kotfic/gw_math_tasks


description = GWFuncDesc.get_description(f)
if description is not None:
for arg in reversed(description.arguments):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not loop over description.positional_args and description.keyword_args?

for extension in get_extensions():
if extension not in ('core', 'docker'):
for task in get_extension_tasks(extension).values():
yield task
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get the convenience of just using the task names, but I do think it will be necessary to at least give the user's a signal about what plugin added the task.

@jbeezley
Copy link

jbeezley commented Oct 3, 2018

From a high level, this looks pretty nice. I think I want to try something non-trivial with it before final judgement though.

Prior to this PR gwrun used the default "function" documentation to
provide help for commands.  This is problematic because the function
is actual a Celery Task whose documentation comes from
celery.local.Proxy. This caused the following behavior:

Usage: gwrun [OPTIONS] COMMAND [ARGS]...

Options:
  -o, --output [json|stdout]
  --help                      Show this message and exit.

Commands:
  add              Proxy that evaluates object once.
  subtract         Proxy that evaluates object once.
  multiply         Proxy that evaluates object once.
  divide           Proxy that evaluates object once.

Now we explicitly set the help string to "" if there is no
documentation on the wrapped celery function. If there IS
documentation on the function is is passed through correctly.
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