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 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 > > > > > > > > > >
