BIND 10 #284: Not every 'data' parsing is done with json

[BIND 10 Development]

#284: Not every 'data' parsing is done with json
-------------------------------+--------------------------------------------
      Reporter:  jelte         |        Owner:  UnAssigned
          Type:  defect        |       Status:  reviewing 
      Priority:  minor         |    Milestone:            
     Component:  Unclassified  |   Resolution:            
      Keywords:                |    Sensitive:  0         
Estimatedhours:                |        Hours:            
      Billable:  1             |   Totalhours:            
      Internal:  0             |  
-------------------------------+--------------------------------------------
Changes (by jinmei):

  * internal:  => 0
  * billable:  => 1


Comment:

 Replying to [comment:1 jelte]:
 > fix + lots of tiny data representation fixlets (json parser wants
 true/false instead of True/False, and no comma at the end of lists and
 maps) in branches/trac284 r2441

  - general: I'm not sure why you changed single quote to double-quote
 (although I have no objection to that).  Is it just some style change or
 something inevitable for this ticket?
  - general: I don't know if s/false/False/ is necessary, but I trust
    you on this point:-)
  - same for s/ast.literal_eval/json.loads/ (about this is correct)
  - cmdctl_test.py: isn't it better to imprt sys at the beginning of the
 file?
  - cfgmgr.py: I didn't understand why pprints were commented out.  If they
 are not necessary anymore why not just clean them up?
  - config/module_spec.py
   - better to import at the beginning of the file?
   - not necessarily for this changeset, but I personally see read(-1)
     rather confusing.  Wouldn't it be clearer to pass nothing to
     read()?  Or at the risk of looking redundant we might define a
     constant such as READ_EVERYTHING = -1 and pass it to read()
   - why aren't exceptions in the 'if' case caught?  Also, is it
     okay/safe to ignore the error and blindly initialize module_spec
     with an empty config? (shouldn't we rather fail with reporting an
     error?)
   - is it a good idea to catch all exceptions here?  I wouldn't
     necessarily object to all catch-all catches, but in general we
     should be careful about that, and in (rare) cases it's justified
     I'd like to see an explicit comment about why we need catch
     everything here and why it's justified.
   - we probably need test cases when json.loads fails.

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