You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ariatosca.apache.org by tliron <gi...@git.apache.org> on 2017/04/20 22:56:37 UTC
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
GitHub user tliron opened a pull request:
https://github.com/apache/incubator-ariatosca/pull/107
ARIA-148 CLI display commands
Also includes some typo fixes for the CLI.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-ariatosca ARIA-148-extra-cli-commands
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-ariatosca/pull/107.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #107
----
commit cd5f233db67c17eb765e5b1f0658b1a047b8f18a
Author: Tal Liron <ta...@gmail.com>
Date: 2017-04-20T22:54:47Z
ARIA-148 CLI display commands
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/107#discussion_r115728444
--- Diff: aria/cli/commands/services.py ---
@@ -38,6 +41,53 @@ def services():
pass
+@services.command(name='show',
+ short_help='Display service information')
+@aria.argument('service-name')
+@aria.options.verbose()
+@aria.options.show_all
+@aria.options.show_graph
+@aria.options.show_json
+@aria.options.show_yaml
+@aria.pass_model_storage
+@aria.pass_logger
+def show(service_name, model_storage, all, graph, json, yaml, logger):
+ """Show information for a specific service template
+
+ `SERVICE_NAME` is the name of the service to display information on.
+ """
+ logger.info('Showing service {0}...'.format(service_name))
+ service = model_storage.service.get_by_name(service_name)
+
+ if json or yaml:
+ all = True
--- End diff --
i actually didnt mean the flag but only the variables etc.,
although now that i think of it it might also be better to change the flag, as it might be confusing i.e. users might think it will show all service-templates or so..?
in any case what matters to me more is the variables, the flag can still be `all` to the user, but the "destination" of the variable (configurable in `aria.py`) should be renamed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/107#discussion_r115729149
--- Diff: aria/cli/commands/service_templates.py ---
@@ -43,32 +45,51 @@ def service_templates():
@aria.argument('service-template-name')
@aria.options.verbose()
@aria.pass_model_storage
+@aria.options.show_all
+@aria.options.show_types
+@aria.options.show_json
+@aria.options.show_yaml
@aria.pass_logger
-def show(service_template_name, model_storage, logger):
- """Show information for a specific service templates
+def show(service_template_name, model_storage, all, types, json, yaml, logger):
+ """Show information for a specific service template
`SERVICE_TEMPLATE_NAME` is the name of the service template to show information on.
"""
- logger.info('Showing service template {0}...'.format(service_template_name))
service_template = model_storage.service_template.get_by_name(service_template_name)
- service_template_dict = service_template.to_dict()
- service_template_dict['#services'] = len(service_template.services)
- column_formatters = \
- dict(description=table.trim_formatter_generator(DESCRIPTION_FIELD_LENGTH_LIMIT))
- columns = SERVICE_TEMPLATE_COLUMNS + ['#services']
- table.print_data(columns, service_template_dict, 'Service-template:',
- column_formatters=column_formatters, col_max_width=50)
-
- if service_template_dict['description'] is not None:
- logger.info('Description:')
- logger.info('{0}{1}'.format(service_template_dict['description'].encode('UTF-8') or '',
- os.linesep))
-
- if service_template.services:
- logger.info('Existing services:')
- for service in service_template.services:
- logger.info('\t{0}'.format(service.name))
+ if json or yaml:
--- End diff --
i'm a bit confused by this. so `yaml`, `json`, and `all` all mean all,
and `json` and `yaml` can be used simulatniously but only `json` will get printed.
Re the latter problem, please have a look at `aria.py` for how to make flags mutually-exclusive.
re the former, it might be better to also have the `all` flag as mutually-exclusive with the other two..
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/107#discussion_r115934967
--- Diff: aria/cli/core/aria.py ---
@@ -65,15 +58,33 @@ def __init__(self, *args, **kwargs):
super(MutuallyExclusiveOption, self).__init__(*args, **kwargs)
def handle_parse_result(self, ctx, opts, args):
- if self.mutually_exclusive.intersection(opts) and self.name in opts:
+ if (self.name in opts) and self.mutually_exclusive.keys().intersection(opts):
--- End diff --
seems like `mutually_exclusive` might be a `tuple` now, so `.keys()` would raise an error here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-ariatosca/pull/107
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/107#discussion_r115934893
--- Diff: aria/cli/core/aria.py ---
@@ -40,16 +41,8 @@
class MutuallyExclusiveOption(click.Option):
- """Makes options mutually exclusive. The option must pass a `cls` argument
- with this class name and a `mutually_exclusive` argument with a list of
- argument names it is mutually exclusive with.
-
- NOTE: All mutually exclusive options must use this. It's not enough to
- use it in just one of the options.
- """
-
def __init__(self, *args, **kwargs):
- self.mutually_exclusive = set(kwargs.pop('mutually_exclusive', []))
+ self.mutually_exclusive = kwargs.pop('mutually_exclusive', tuple())
--- End diff --
why change this from a set to another type?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/107#discussion_r115726284
--- Diff: aria/cli/table.py ---
@@ -85,7 +85,7 @@ def get_values_per_column(column, row_data):
return val
else:
- return defaults[column]
+ return defaults.get(column) if defaults is not None else None
--- End diff --
should instead have a `defaults = defaults or {}` before calling the nested method
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/107#discussion_r114553081
--- Diff: aria/cli/helptexts.py ---
@@ -47,3 +47,8 @@
SORT_BY = "Key for sorting the list"
DESCENDING = "Sort list in descending order [default: False]"
JSON_OUTPUT = "Output logs in a consumable JSON format"
+
+DISPLAY_JSON = "Display in JSON format"
--- End diff --
lets merge this with `JSON_OUTPUT`,
the latter is a good name, and simply have it as "Format the output as JSON"
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/107#discussion_r114554248
--- Diff: aria/cli/commands/services.py ---
@@ -177,3 +179,31 @@ def inputs(service_name, model_storage, logger):
logger.info(inputs_string.getvalue())
else:
logger.info('\tNo inputs')
+
+
+@services.command(name='display',
+ short_help='Display service information')
+@aria.argument('service-name')
+@aria.options.verbose()
+@aria.options.display_json
+@aria.options.display_yaml
+@aria.options.display_graph
+@aria.pass_model_storage
+@aria.pass_logger
+def display(service_name, model_storage, json, yaml, graph, logger):
--- End diff --
nitpicking that model_storage should probably sit after `graph`, before `logger`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/107#discussion_r114553136
--- Diff: aria/cli/core/aria.py ---
@@ -325,6 +325,30 @@ def __init__(self):
default=defaults.SERVICE_TEMPLATE_FILENAME,
help=helptexts.SERVICE_TEMPLATE_FILENAME)
+ self.display_json = click.option(
--- End diff --
merge this with the other JSON one
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/107#discussion_r115935686
--- Diff: aria/cli/core/aria.py ---
@@ -65,15 +58,33 @@ def __init__(self, *args, **kwargs):
super(MutuallyExclusiveOption, self).__init__(*args, **kwargs)
def handle_parse_result(self, ctx, opts, args):
- if self.mutually_exclusive.intersection(opts) and self.name in opts:
+ if (self.name in opts) and self.mutually_exclusive.keys().intersection(opts):
raise click.UsageError(
'Illegal usage: `{0}` is mutually exclusive with '
'arguments: [{1}] ({2}).'.format(
self.name,
self.mutuality_string,
self.mutuality_error_message))
- return super(MutuallyExclusiveOption, self).handle_parse_result(
- ctx, opts, args)
+ return super(MutuallyExclusiveOption, self).handle_parse_result(ctx, opts, args)
+
+
+def mutually_exclusive_option(*param_decls, **attrs):
+ """
+ Makes options mutually exclusive. The decorator must pass a a ``mutually_exclusive`` argument
+ with a list of argument names with which the option is mutually exclusive.
+
+ NOTE: All mutually exclusive options must use this. It's not enough to use it in just one of the
+ options.
+ """
+ def decorator(func):
+ if 'help' in attrs:
--- End diff --
could you please document this func a bit more?
I'm not sure what are `param_decls` and `attrs` in this case, why `help` is being inspected, and why the internal `__click_params__` is accessed directly here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/107#discussion_r115934648
--- Diff: aria/cli/core/aria.py ---
@@ -325,6 +339,46 @@ def __init__(self):
default=defaults.SERVICE_TEMPLATE_FILENAME,
help=helptexts.SERVICE_TEMPLATE_FILENAME)
+ self.show_full = click.option(
+ '-f',
+ '--full',
+ 'mode',
+ is_flag=True,
+ flag_value='full',
--- End diff --
I think I don't like this approach as much at the previous one.
It seems like it simplifies the code a bit but makes things somewhat more confusing to the user - specifically the fact that only the last flag of a given "destination" (e.g. `mode`) takes hold.
If instead you have different destinations for these flags, it'd be easier to inform the user of misuse (even if for some reason the mutually-exclusive mechanism is problematic to use, you can always just manually validate the parameters in the command function itself)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/107#discussion_r114554090
--- Diff: aria/cli/commands/services.py ---
@@ -177,3 +179,31 @@ def inputs(service_name, model_storage, logger):
logger.info(inputs_string.getvalue())
else:
logger.info('\tNo inputs')
+
+
+@services.command(name='display',
+ short_help='Display service information')
+@aria.argument('service-name')
+@aria.options.verbose()
+@aria.options.display_json
+@aria.options.display_yaml
+@aria.options.display_graph
+@aria.pass_model_storage
+@aria.pass_logger
+def display(service_name, model_storage, json, yaml, graph, logger):
+ """Display information for a specific service template
+
+ `SERVICE_NAME` is the name of the service to display information on.
+ """
+ logger.info('Displaying service {0}...'.format(service_name))
+ service = model_storage.service.get_by_name(service_name)
+ consumption.ConsumptionContext()
+ if graph:
--- End diff --
General reminder that none of the methods below are standard for the CLI
Fine for now, but keep https://issues.apache.org/jira/browse/ARIA-183 in mind
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/107#discussion_r115935090
--- Diff: aria/cli/core/aria.py ---
@@ -65,15 +58,33 @@ def __init__(self, *args, **kwargs):
super(MutuallyExclusiveOption, self).__init__(*args, **kwargs)
def handle_parse_result(self, ctx, opts, args):
- if self.mutually_exclusive.intersection(opts) and self.name in opts:
+ if (self.name in opts) and self.mutually_exclusive.keys().intersection(opts):
raise click.UsageError(
'Illegal usage: `{0}` is mutually exclusive with '
'arguments: [{1}] ({2}).'.format(
self.name,
self.mutuality_string,
self.mutuality_error_message))
- return super(MutuallyExclusiveOption, self).handle_parse_result(
- ctx, opts, args)
+ return super(MutuallyExclusiveOption, self).handle_parse_result(ctx, opts, args)
+
+
+def mutually_exclusive_option(*param_decls, **attrs):
+ """
+ Makes options mutually exclusive. The decorator must pass a a ``mutually_exclusive`` argument
--- End diff --
a a a a a
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/107#discussion_r114555739
--- Diff: aria/cli/commands/service_templates.py ---
@@ -181,21 +184,49 @@ def validate(service_template, service_template_filename,
logger.info('Service template validated successfully')
+@service_templates.command(name='display',
+ short_help='Display service template information')
+@aria.argument('service-template-name')
+@aria.options.verbose()
+@aria.options.display_json
+@aria.options.display_yaml
+@aria.options.display_types
+@aria.pass_model_storage
+@aria.pass_logger
+def display(service_template_name, model_storage, json, yaml, types, logger):
--- End diff --
Having both `display` and `show` is confusing :/
I agree however that it doesn't make sense to make this command into a mere flag of `show` as they're very different.
I'm generally ok with this as is, but Maxim wanted me to suggest `aria parse service/service-template` as an option as well. I'll leave it for your consideration :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/107#discussion_r114553193
--- Diff: aria/cli/core/aria.py ---
@@ -325,6 +325,30 @@ def __init__(self):
default=defaults.SERVICE_TEMPLATE_FILENAME,
help=helptexts.SERVICE_TEMPLATE_FILENAME)
+ self.display_json = click.option(
+ '-j',
+ '--json',
+ is_flag=True,
+ help=helptexts.DISPLAY_JSON)
+
+ self.display_yaml = click.option(
--- End diff --
`yaml_output`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #107: ARIA-148 CLI display commands
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/107#discussion_r115726390
--- Diff: aria/cli/commands/services.py ---
@@ -38,6 +41,53 @@ def services():
pass
+@services.command(name='show',
+ short_help='Display service information')
+@aria.argument('service-name')
+@aria.options.verbose()
+@aria.options.show_all
+@aria.options.show_graph
+@aria.options.show_json
+@aria.options.show_yaml
+@aria.pass_model_storage
+@aria.pass_logger
+def show(service_name, model_storage, all, graph, json, yaml, logger):
+ """Show information for a specific service template
+
+ `SERVICE_NAME` is the name of the service to display information on.
+ """
+ logger.info('Showing service {0}...'.format(service_name))
+ service = model_storage.service.get_by_name(service_name)
+
+ if json or yaml:
+ all = True
--- End diff --
i'd rename the flag, it's both unclear and a python keyword. `show_all` or `all_info` might be better.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---