cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rohit Yadav <bhais...@apache.org>
Subject Re: cloudmonkey printing enhancements proposal
Date Mon, 01 Apr 2013 08:07:53 GMT
Hi Justin, thanks for your first patch.

As per our workflows, we use git for our version control and have a
reviewboard for reviewing patches and an issue tracker for tracking
bugs/features.

Git:
https://cwiki.apache.org/confluence/display/CLOUDSTACK/Git

Review board:
https://reviews.apache.org

For filtering only passed key names, we can cut down the whole if-else tree
using filter (which you may see I'd used at several places) and reduce list.

Something like; filter(lamba key: <put here condition to evaluate key, like
key in my set of filters>, dictionary.keys()), using this filtered failsafe
list of keys, we can generate dictionary, using a map; map(lambda x:
myjson[x] = jsondictionary[x], failsafekeys) etc.

I can help with this, thanks for your patch. Since it's small I can help
apply that for future please use review board or share git formatted patch
which would apply cleanly on master.

Regards.

On Mon, Apr 1, 2013 at 10:42 AM, Justin Grudzien <grudzien@gmail.com> wrote:

> Greetings,
>
> I am posting my first patch. I am not sure exactly how to submit a patch
> but I figure I will paste it here and someone will tell me the right
> method. I am keeping track of the work that I am doing and as Rohit
> suggested I will update the mailing list as I progress. This first patch
> adds basic JSON output with filtering. I have tested it on cloudmonkey
> 4.1.0-0 and 4.1.0-snapshot3 and all seems well. Comments are appreciated.
>
> --- README ---
> Added
> 1. display = [default|json|tabularize] has been added in the config to
> replace
>     tabularize = [true|false]
> 2. tabularize is deprecated but we will still set it as "false" once the
> user
>     removes it out of their config to avoid throwing an error. This will be
>     removed in the next major version.
> 3. display = "default" is added to the [ui] section of the config if it is
> not
>     present.
> 4. You can now output JSON formatted text by setting the config display =
> json
> 5. You can now filter text in JSON output mode. (i.e. list users
>     account=grudzien filter=account,id,email). Filtered output returns a
>     properly formatted JSON document.
>
> Removed
> 1. Removed the printing of attr keys in read_config().
>
> Deprecated
> 1. tabularize = [true|false] is now messaged as deprecated.
>
> ToDo
> 1. Need to format empty return messages with proper JSON messaging.
> 2. Need to standardize JSON output messaging.
>     A. Add return code [Error|Success] to all calls.
>     B. Add called API verb.
> 3. Count is not decreased in results when filtering completely eliminates a
>     result.
> 4. JSON print needs to be implemented manually to support colors. Right now
>     json.dumps() pretty prints the output.
> 5. Color tagging needs to be added to JSON printing.
> 6. Error messages need to have proper JSON formatting.
> 7. Various help text has grammar or consistency errors that need fixing.
> 8. Make color a passable option to a command instead of a config parameter.
>     (i.e. list users account=grudzien color=true)
>     A. This will require reworking the result_filter passable argument to
> the
>         various print methods since they only expect filter= input.
> 9. The API in CloudStack 4.0.1 does not all return proper JSON formatted
> text.
>     As a result some of the JSON printing looks funny. I will get the later
>     versions into a lab and test them to make sure the formatting bugs are
> in
>     newer revisions and submit them as bugs.
> --- README ---
>
> --- PATCH ---
> diff -rupN cloudstack/tools/cli/cloudmonkey/cloudmonkey.py
> cloudstackmodified/tools/cli/cloudmonkey/cloudmonkey.py
> --- cloudstack/tools/cli/cloudmonkey/cloudmonkey.py 2013-03-31
> 16:58:26.000000000 -0500
> +++ cloudstackmodified/tools/cli/cloudmonkey/cloudmonkey.py 2013-03-31
> 22:03:38.000000000 -0500
> @@ -27,6 +27,7 @@ try:
>      import shlex
>      import sys
>      import types
> +    import copy
>
>      from cachemaker import loadcache, savecache, monkeycache,
> splitverbsubject
>      from config import __version__, __description__, __projecturl__
> @@ -162,6 +163,44 @@ class CloudMonkeyShell(cmd.Cmd, object):
>                  self.monkeyprint(printer)
>              return PrettyTable(toprow)
>
> +        # method: print_result_json( result, result_filter )
> +        # parameters: result - raw results from the API call
> +        #             result_filter - filterset
> +        # description: prints result as a json object
> +        def print_result_json(result, result_filter=None):
> +            tfilter = {} # temp var to hold a dict of the filters
> +            tresult = copy.deepcopy(result) # dupe the result to filter
> +            if result_filter != None:
> +                for res in result_filter:
> +                    tfilter[ res ] = 1
> +                myresults = {}
> +                for okey, oval in result.iteritems():
> +                    if isinstance( oval, dict ):
> +                        for tkey in x:
> +                            if tkey not in tfilter:
> +                                try:
> +                                    del( tresult[okey][x][tkey] )
> +                                except:
> +                                    pass
> +                    elif isinstance( oval, list ):
> +                        for x in range( len( oval ) ):
> +                            if isinstance( oval[x], dict ):
> +                                for tkey in oval[x]:
> +                                    if tkey not in tfilter:
> +                                        try:
> +                                            del( tresult[okey][x][tkey] )
> +                                        except:
> +                                            pass
> +                            else:
> +                                try:
> +                                    del( tresult[ okey ][ x ] )
> +                                except:
> +                                    pass
> +            print json.dumps(tresult,
> +                             sort_keys=True,
> +                             indent=2,
> +                             separators=(',', ': '))
> +
>          def print_result_tabular(result, result_filter=None):
>              toprow = None
>              printer = None
> @@ -183,6 +222,12 @@ class CloudMonkeyShell(cmd.Cmd, object):
>                  self.monkeyprint(printer)
>
>          def print_result_as_dict(result, result_filter=None):
> +
> +            # tabularize overrides self.display
> +            if self.display == "json" and not self.tabularize == "true":
> +                print_result_json(result, result_filter)
> +                return
> +
>              for key in sorted(result.keys(), key=lambda x:
>                                x not in ['id', 'count', 'name'] and x):
>                  if not (isinstance(result[key], list) or
> @@ -195,7 +240,7 @@ class CloudMonkeyShell(cmd.Cmd, object):
>          def print_result_as_list(result, result_filter=None):
>              for node in result:
>                  # Tabular print if it's a list of dict and tabularize is
> true
> -                if isinstance(node, dict) and self.tabularize == 'true':
> +                if isinstance(node, dict) and (self.display ==
> 'tabularize' or self.tabularize == 'true'):
>                      print_result_tabular(result, result_filter)
>                      break
>                  self.print_result(node)
> @@ -318,7 +363,7 @@ class CloudMonkeyShell(cmd.Cmd, object):
>                      autocompletions = uuids
>                      search_string = value
>
> -        if self.tabularize == "true" and subject != "":
> +        if (self.display == "tabularize" or self.display == "json" or
> self.tabularize == "true") and subject != "":
>              autocompletions.append("filter=")
>          return [s for s in autocompletions if s.startswith(search_string)]
>
> @@ -459,7 +504,6 @@ class CloudMonkeyShell(cmd.Cmd, object):
>          self.monkeyprint("Bye!")
>          return self.do_EOF(args)
>
> -
>  class MonkeyParser(OptionParser):
>      def format_help(self, formatter=None):
>          if formatter is None:
> @@ -473,7 +517,6 @@ class MonkeyParser(OptionParser):
>          result.append("\nTry cloudmonkey [help|?]\n")
>          return "".join(result)
>
> -
>  def main():
>      parser = MonkeyParser()
>      parser.add_option("-c", "--config-file",
> diff -rupN cloudstack/tools/cli/cloudmonkey/config.py
> cloudstackmodified/tools/cli/cloudmonkey/config.py
> --- cloudstack/tools/cli/cloudmonkey/config.py 2013-03-31
> 16:58:26.000000000 -0500
> +++ cloudstackmodified/tools/cli/cloudmonkey/config.py 2013-03-31
> 23:09:01.000000000 -0500
> @@ -56,7 +56,8 @@ config_fields['core']['log_file'] = expa
>  # ui
>  config_fields['ui']['color'] = 'true'
>  config_fields['ui']['prompt'] = '> '
> -config_fields['ui']['tabularize'] = 'false'
> +config_fields['ui']['tabularize'] = 'false' # deprecate - REMOVE
> +config_fields['ui']['display'] = 'default' # default display mechanism
>
>  # server
>  config_fields['server']['host'] = 'localhost'
> @@ -111,9 +112,19 @@ def read_config(get_attr, set_attr, conf
>      for section in config_fields.keys():
>          for key in config_fields[section].keys():
>              try:
> +                if( key == "tabularize" ): # this key is deprecated
> +                    print "\ntabularize config parameter is deprecated:",
> +                    print "please switch to display =",
> +                    print "[default,json,tabularize]\n"
>                  set_attr(key, config.get(section, key))
>              except Exception:
> -                missing_keys.append(key)
> +                if( key == "tabularize" ): # this key is deprecated
> +                    set_attr( key, "false" ) # set default
> +                elif( key == "display" ): # this key is deprecated
> +                    config = write_config(get_attr, config_file, True)
> +                    set_attr( key, "default" ) # set default
> +                else:
> +                    missing_keys.append(key)
>
>      if len(missing_keys) > 0:
>          print "Please fix `%s` in %s" % (', '.join(missing_keys),
> config_file)
> --- PATCH ---
>
> Justin
>
>
> On Sun, Mar 31, 2013 at 2:15 AM, Rohit Yadav <bhaisaab@apache.org> wrote:
>
> > Hey Justin, thanks for posting this on community ML.
> >
> > On Sat, Mar 30, 2013 at 9:20 AM, Justin Grudzien <grudzien@gmail.com>
> > wrote:
> >
> > > My company is building a private cloud and we are moving to cloudstack.
> > As
> > > we begun investigating the cloudmonkey CLI we found that the output was
> > > slightly hard to read. I have begun working on some optimizations that
> I
> > > think will benefit the community and I reached out to Rohit, who
> > > recommended that I join this list and present my ideas. Here is what I
> am
> > > proposing:
> > >
> > > 1. Add json output to cloudmonkey
> > > I have accomplished this by adding a config parameter called display,
> > which
> > > can be set to json, tabularize, or default. I have removed the
> tabularize
> > > parameter.
> > >
> >
> > +1
> >
> >
> > >
> > > Justins-MacBook-Pro:cloudmonkey grudzien$ cloudmonkey list users
> > > account=grudzien
> > > {
> > >   "count": 1,
> > >   "user": [
> > >     {
> > >       "account": "grudzien",
> > >       "accountid": "b799783d-e5bb-460a-be0e-3966bd69edda",
> > >       "accounttype": 1,
> > >       "apikey": "*nokey*",
> > >       "created": "2013-03-27T16:09:17-0500",
> > >       "domain": "ROOT",
> > >       "domainid": "7e61c32f-9873-4944-947a-dcc00d3bebdc",
> > >       "email": "grudzien@gmail.com",
> > >       "firstname": "Justin",
> > >       "id": "265930bc-62ef-41f8-849c-e58593ca4b1f",
> > >       "lastname": "Grudzien",
> > >       "secretkey": "*nokey*",
> > >       "state": "enabled",
> > >       "username": "grudzien"
> > >     }
> > >   ]
> > > }
> > >
> > > 2. Add filtering as a standard parameter for all output types.
> > > The only thing that has filtering now is the tabular output and grep
> > breaks
> > > the json.
> > >
> > > Justins-MacBook-Pro:cloudmonkey grudzien$ cloudmonkey list users
> > > account=grudzien filter=account,email,username,state
> > > {
> > >   "count": 1,
> > >   "user": [
> > >     {
> > >       "account": "grudzien",
> > >       "email": "grudzien@gmail.com",
> > >       "state": "enabled",
> > >       "username": "grudzien"
> > >     }
> > >   ]
> > > }
> > >
> >
> > Awesome.
> >
> >
> > >
> > > 3. Add color to the json output
> > > I was thinking of colorizing the keys in the key/value pairs to
> increase
> > > readability.
> > >
> >
> > This is easily doable, all we have to do is define a regex rule in
> > printer.py for the regex type "txt": as key and ": xxx, as value.
> >
> >
> > > 4. Move the color option from the config file to the command line.
> > > There are two reasons for this. First, I want to be able to wrap a
> script
> > > around cloudmonkey and not have to worry about colorization that will
> > > impede me processing the output and second I think it would be more
> > useful
> > > to use the highlighting on demand rather than having to back out of the
> > > shell to edit a config file.
> > >
> >
> > I did not understand this one. What exactly are you proposing? You can
> > always set color to false or true. Are you talking about color themes?
> >
> >
> > > 5. Standardize messaging for the output types.
> > > Right now certain kinds of messaging is presented differently for an
> > output
> > > type. For example, if I issue an api command that doesn't exist it
> > displays
> > > a generic error message, regardless of the output type selected.
> Ideally,
> > > all output would be in the specified format.
> > >
> >
> > Oh man, that would be just great! Pl. share your patch.
> >
> >
> > >
> > > I have the first two working and am planning on implementing the others
> > as
> > > I flesh them out. I will submit a patch when I feel it is ready. Any
> > early
> > > feedback on whether these changes will be useful to others is
> > appreciated.
> > >
> >
> > You can submit patches as you finish them, don't wait till all of 5
> things
> > are done. I can help review them and merge them.
> >
> > Thanks for your proposal and sharing them.
> >
> > Cheers.
> >
> >
> >
> > > Justin
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message