Rohit, I submitted my first patch to review board. Can you please take a look when you get a moment I want to make sure I did everything correctly. Also, other than submit to review board do I need to forward anything to the dev mailing list saying that I have a review request or will the reviewers just see the automated messages?
The patch right now doesn't use the lambda function when filtering but the next version should have that incorporated when I add more functionality. Thanks or all your help. Justin Sent from my iPhone On Apr 1, 2013, at 1:41 PM, Rohit Yadav <bhais...@apache.org> wrote: > On Mon, Apr 1, 2013 at 4:26 PM, Justin Grudzien <grudz...@gmail.com> 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. 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. > > Okay, let me see them if I can get some time tomorrow. > > About keep it simple, I think it's a matter of art. IMO such thinkings has > been exploited by people to write a lot of verbose code. I think we're > artists and our art evolves with time and so does our tools and techniques. > > I think it's time for functional programming, for example we now have > anonymous functions or lambdas in mainstream langs (or their iterations) > such as C++11 and Java8. There is really a use of such a paradigms, we're > not throwing them just because lambdas are cool (even though lamba calculus > gives you your eureka moments :) > > Regards. > PS. An iterative programmer may think an OOP programmer writes hard to read > code and an OOP programmer may think the same for a functional programmer, > a functional programmer thinks nobody in the world knows how to really > program :P Lastly, a user does not care who're coding it as long as it > works lol :P > > >> 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 >>
