BIND 10 #22: MsgQ review

[BIND 10 Development]

#22: MsgQ review
--------------------------+-------------------------------------------------
      Reporter:  mgraff   |        Owner:  jelte                      
          Type:  task     |       Status:  reviewing                  
      Priority:  blocker  |    Milestone:  06. 4th Incremental Release
     Component:  msgq     |   Resolution:                             
      Keywords:  review   |    Sensitive:  0                          
Estimatedhours:           |        Hours:                             
      Billable:  0        |   Totalhours:                             
      Internal:  0        |  
--------------------------+-------------------------------------------------
Changes (by hanfeng):

  * owner:  hanfeng => jelte
  * estimatedhours:  => 0.0
  * internal:  => 0
  * billable:  => 0


Comment:

 1 in file session.py line 87, if the length of the "env" is really big
 which is greater than the limit of unsigned short, the "env" will be
 truncated, is it better to raise an exception?

 2 in file session.py line 115 to line 117, I don't know the purpose of
 these code, it seems useless.

 3 in the session, there is one queue_ to store the message sent through
 message channel, if some one only sent and no one pick up the message from
 the queue, will it be a problem for memory usage?

 4 trivial coding style:
 4.1 in file data.py line 34
   for ka in a.keys():
      if ka in b and a[ka] == b[ka]:
          to_remove.append(ka)
 if use list comprehension in python, this part code can be written as
 follows which is cleaner
   duplicate_keys = [key for key in a.keys() if key in b and a[key] ==
 b[key]]

 4.2 in file data.py line 40, function merge, python already provide
 function update for dictionary , the difference between merge and
 update,is that merge will remove null value items. so for merge, it can be
 written as follows which maybe cleaner
     def merge(orig, new):
      if type(orig) != dict or type(new) != dict:
       raise DataTypeError("Not a dict in merge()")
      orig.update(new)
      remove_null_items(orig)
     def remove_null_items(d):
       for key in d.keys():
        if type(d[key]) == dict:
           remove_null_items(d[key])
        else if not d[key]:
           del d[key]

-- 
Ticket URL: <http://bind10.isc.org/ticket/22#comment:14>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development