cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Grudzien <grudz...@gmail.com>
Subject Re: cloudmonkey printing enhancements proposal
Date Tue, 02 Apr 2013 03:21:34 GMT
I created CLOUDSTACK-1875 for the JSON enhancements. I am going to begin
filling out the ticket with the work that I have done and I will submit the
first patch as soon as I figure out the rest of the non-comitter steps. I
should be able to make my first patch submission attempt tomorrow. Thanks
for all the help so far.

Justin


On Mon, Apr 1, 2013 at 8:52 AM, Chip Childers <chip.childers@sungard.com>wrote:

> On Mon, Apr 01, 2013 at 05:56:11AM -0500, Justin Grudzien wrote:
> > 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.
>
> Go for it Justin.
>
> Also, give me your Jira ID once you have one, so that I can give you
> karma to assign the "improvement" bug to yourself (and resolve it once
> the patch is committed).
>
> > 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
> > 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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message