> -----Original Message----- > From: Justin Grudzien [mailto:grudz...@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 <bhais...@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 <grudz...@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 <bhais...@apache.org> > > wrote: > > > > > > > Hey Justin, thanks for posting this on community ML. > > > > > > > > On Sat, Mar 30, 2013 at 9:20 AM, Justin Grudzien > > > > <grudz...@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": "grudz...@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": "grudz...@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 > > > > > > > > > > > > > >
