Conversation
Contributor
Author
|
To test this I recommend the following: mkdir gwruntest; cd gwruntest
# Make a virtualenv
mkvirtualenv -a . gwruntest
# Clone the repos
git clone -b gwrun [email protected]:girder/girder_worker.git
git clone -b gwrun_argspec [email protected]: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 --helpNote there is an implementation of a toy plugin at kotfic/gw_math_tasks |
jbeezley
reviewed
Oct 3, 2018
jbeezley
reviewed
Oct 3, 2018
|
|
||
| description = GWFuncDesc.get_description(f) | ||
| if description is not None: | ||
| for arg in reversed(description.arguments): |
There was a problem hiding this comment.
Why not loop over description.positional_args and description.keyword_args?
jbeezley
reviewed
Oct 3, 2018
| for extension in get_extensions(): | ||
| if extension not in ('core', 'docker'): | ||
| for task in get_extension_tasks(extension).values(): | ||
| yield task |
There was a problem hiding this comment.
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.
|
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
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
Finally
The
cli_argsandcli_optsimplementation (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:represents a much nicer API and a mapping could be used to negotiate the difference between
type=floatandcli_opts={'type': click.FLOAT}behind the scenes. This serves the purpose of: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.