cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Edison Su <Edison...@citrix.com>
Subject RE: cloudmonkey printing enhancements proposal
Date Mon, 01 Apr 2013 17:43:48 GMT


> -----Original Message-----
> From: Justin Grudzien [mailto:grudzien@gmail.com]
> Sent: Monday, April 01, 2013 3:56 AM
> To: dev@cloudstack.apache.org
> Subject: Re: cloudmonkey printing enhancements proposal
>
> Thanks for the information. If you haven't already put the patch for review I
> would like to do so and follow the directions so I can get used to the process.
> Let me know. Also, I saw the lambda functions and chose not to use them
> because I think they make the code harder to read for the more novice
> python programmer. I can go back and add it if the goal is to have shorter

+2, I am amateur python programmer, hate to read obscure code, especially if the language
is no type...

> code but if it were up to me I would have the code more readable. :) I hope
> people find this work to be useful, I appreciate the guidance through the
> submission process.
>
> Justin
>
>
> On Mon, Apr 1, 2013 at 3:07 AM, Rohit Yadav <bhaisaab@apache.org> wrote:
>
> > 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
View raw message