• 
      

    Handle typed values much better in get-siteconfig and set-siteconfig.

    Review Request #7311 — Created May 17, 2015 and submitted

    Information

    Djblets
    release-0.8.x
    1c27326...

    Reviewers

    Our siteconfig management commands did pretty well for string values, but they
    had several issues for other types:

    • get-siteconfig would fail with an exception for any value that wasn't a
      string.
    • set-siteconfig wouldn't understand anything but "0" and "1" for boolean
      values, despite list-siteconfig printing "true" and "false".
    • set-siteconfig was unable to set a key to None.

    I've made several changes to address these issues. The one complication is that
    in our type detection, for any type that doesn't have a default value, we have
    to make a leap of faith that the passed-in value should be a string. This isn't
    terribly common but does show up with some of the LDAP attributes.

    Ran through all test cases listed in the attached bug.

    Description From Last Updated

    Col: 29 E721 do not compare types, use 'isinstance()'

    reviewbotreviewbot

    Can we normalize value and call sys.stdout.write only once?

    chipx86chipx86

    Should use tuples instead of lists, for consistency (and expremely small performance/memory gains).

    chipx86chipx86

    Could simplify to: defaults = siteconfig.get_defaults() value_type = type(defaults.get(key_basename, ''))

    chipx86chipx86
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          djblets/siteconfig/management/commands/get-siteconfig.py
          djblets/siteconfig/management/commands/set-siteconfig.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          djblets/siteconfig/management/commands/get-siteconfig.py
          djblets/siteconfig/management/commands/set-siteconfig.py
      
      
    2. Show all issues
      Col: 29
       E721 do not compare types, use 'isinstance()'
      
    3. 
        
    david
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          djblets/siteconfig/management/commands/get-siteconfig.py
          djblets/siteconfig/management/commands/set-siteconfig.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          djblets/siteconfig/management/commands/get-siteconfig.py
          djblets/siteconfig/management/commands/set-siteconfig.py
      
      
    2. 
        
    chipx86
    1. 
        
    2. djblets/siteconfig/management/commands/get-siteconfig.py (Diff revision 2)
       
       
       
       
       
       
       
       
       
       
      Show all issues

      Can we normalize value and call sys.stdout.write only once?

    3. Show all issues

      Should use tuples instead of lists, for consistency (and expremely small performance/memory gains).

    4. djblets/siteconfig/management/commands/set-siteconfig.py (Diff revision 2)
       
       
       
       
       
       
      Show all issues

      Could simplify to:

      defaults = siteconfig.get_defaults()
      value_type = type(defaults.get(key_basename, ''))
      
    5. 
        
    david
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          djblets/siteconfig/management/commands/get-siteconfig.py
          djblets/siteconfig/management/commands/set-siteconfig.py
      
      
      
      Tool: Pyflakes
      Processed Files:
          djblets/siteconfig/management/commands/get-siteconfig.py
          djblets/siteconfig/management/commands/set-siteconfig.py
      
      
    2. 
        
    chipx86
    1. Ship It!
    2. 
        
    david
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-0.8.x (9e1e462)