#8951 NORM Not Tri: wpa support for NM07 in sugar head

[Zarro Boogs per Child]

#8951: wpa support for NM07 in sugar head
------------------------------+---------------------------------------------
           Reporter:  erikos  |       Owner:  marco        
               Type:  defect  |      Status:  new          
           Priority:  normal  |   Milestone:  Not Triaged  
          Component:  sugar   |     Version:  not specified
         Resolution:          |    Keywords:  r?           
        Next_action:  review  |    Verified:  0            
Deployment_affected:          |   Blockedby:               
           Blocking:          |  
------------------------------+---------------------------------------------

Comment(by tomeu):

 {{{
 +            # No security
 +            return
 }}}

 I would prefer to explicitly return None, and test for that value in the
 callers. Though the most usual solution would be to return a tuple or an
 object with those properties as members.

 {{{
 +            if security:
 }}}

 This construction should be used only when security is a boolean or a
 sequence. If you want to check if it's None, just do "if security is
 None".

 {{{
 +                key = 'key-mgmt'
 +                if self._settings['802-11-wireless-
 security'].has_key(key):
 +                    value = self._settings['802-11-wireless-
 security'][key]
 +                    config.set(identifier, key, value)
 +                key = 'proto'
 +                if self._settings['802-11-wireless-
 security'].has_key(key):
 +                    value = self._settings['802-11-wireless-
 security'][key]
 +                    config.set(identifier, key, value)
 +                key = 'pairwise'
 +                if self._settings['802-11-wireless-
 security'].has_key(key):
 +                    value = self._settings['802-11-wireless-
 security'][key]
 +                    config.set(identifier, key, value)
 +                key = 'group'
 +                if self._settings['802-11-wireless-
 security'].has_key(key):
 +                    value = self._settings['802-11-wireless-
 security'][key]
 +                    config.set(identifier, key, value)
 }}}

 Couldn't this be done in a nice "for key in ['key-mgmt', 'proto', ...]:"
 loop?

 {{{
 +        else:
 +            f = open(config_path, 'w')
 +            try:
 +                config.write(f)
 +            except ConfigParser.Error, e:
 +                logging.error('Can not write %s error: %s' %
 (config_path, e))
 +            f.close()
 }}}

 What this else clause does?

 {{{
 +        settings = {'connection': {'id': section}}
 }}}

 I'm a bit concerned about all these strings in the code, cannot we use a
 python object with some properties instead?

 Globally, this patch sounds pretty good to me.

-- 
Ticket URL: <http://dev.laptop.org/ticket/8951#comment:1>
One Laptop Per Child <http://laptop.org/>
OLPC bug tracking system