Ticket #2 (closed defect: fixed)

Opened 5 years ago

Last modified 5 years ago

wpa support for NM07 in sugar head

Reported by: erikos Owned by: erikos
Priority: major Milestone:
Component: sugar Version:
Severity: Keywords: r+
Cc: morgan.collett@… Distribution/OS:
Bug Status:

Description

- adds the loading and saving of the connections file

- support of wpa

- cleaning up the dialogs (looked what nm-applet is doing)

Attachments

nm07_wpa.patch Download (28.0 KB) - added by erikos 5 years ago.
does include better error handling for reading the config file and move the settings and secrets to python objects

Change History

Changed 5 years ago by erikos

tomeu comments:

+            # 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.

Changed 5 years ago by marcopg

  • owner changed from tomeu to marcopg
  • component changed from glucose to sugar

Changed 5 years ago by erikos

  • owner changed from marcopg to erikos
  • status changed from new to assigned

Changed 5 years ago by morgs

  • cc morgan.collett@… added

Changed 5 years ago by erikos

does include better error handling for reading the config file and move the settings and secrets to python objects

Changed 5 years ago by erikos

  • keywords r? added

Changed 5 years ago by tomeu

  • keywords r+ added; r? removed

Though we should make translatable that 'Auto ' string and try to keep abbreviations to the minimum possible.

Changed 5 years ago by erikos

  • status changed from assigned to closed
  • resolution set to fixed

Done and pushed - thanks for the review!

Note: See TracTickets for help on using tickets.