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