Some more (spring) cleaning.

gns3server/compute/notification_manager.py

@@ -15,16 +15,15 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-from contextlib import contextmanager
-
 
+from contextlib import contextmanager
 from ..notification_queue import NotificationQueue
 
 
 class NotificationManager:
     """
     Manage the notification queue where the controller
-    will connect to get notifications from computes
+    will connect to get notifications from compute servers
     """
 
     def __init__(self):
@@ -48,7 +47,7 @@ class NotificationManager:
 
         :param action: Action name
         :param event: Event to send
-        :param kwargs: Add this meta to the notif (project_id for example)
+        :param kwargs: Add this meta to the notification (project_id for example)
         """
         for listener in self._listeners:
             listener.put_nowait((action, event, kwargs))

gns3server/compute/port_manager.py

@@ -24,8 +24,11 @@ import logging
 log = logging.getLogger(__name__)
 
 
-# This ports are disallowed by Chrome and Firefox to avoid trouble with skip them
-BANNED_PORTS = set((1, 7, 9, 11, 13, 15, 17, 19, 20, 21, 22, 23, 25, 37, 42, 43, 53, 77, 79, 87, 95, 101, 102, 103, 104, 109, 110, 111, 113, 115, 117, 119, 123, 135, 139, 143, 179, 389, 465, 512, 513, 514, 515, 526, 530, 531, 532, 540, 556, 563, 587, 601, 636, 993, 995, 2049, 3659, 4045, 6000, 6665, 6666, 6667, 6668, 6669))
+# This ports are disallowed by Chrome and Firefox to avoid issues, we skip them as well
+BANNED_PORTS = set((1, 7, 9, 11, 13, 15, 17, 19, 20, 21, 22, 23, 25, 37, 42, 43, 53, 77, 79, 87, 95, 101, 102, 103,
+                    104, 109, 110, 111, 113, 115, 117, 119, 123, 135, 139, 143, 179, 389, 465, 512, 513, 514, 515, 526,
+                    530, 531, 532, 540, 556, 563, 587, 601, 636, 993, 995, 2049, 3659, 4045, 6000, 6665, 6666, 6667,
+                    6668, 6669))
 
 
 class PortManager:
@@ -106,7 +109,7 @@ class PortManager:
         return self._udp_host
 
     @udp_host.setter
-    def host(self, new_host):
+    def udp_host(self, new_host):
 
         self._udp_host = new_host
 

gns3server/compute/project.py

@@ -201,8 +201,7 @@ class Project:
 
     def _update_temporary_file(self):
         """
-        Update the .gns3_temporary file in order to reflect current
-        project status.
+        Update the .gns3_temporary file in order to reflect the current project status.
         """
 
         if not hasattr(self, "_path"):
@@ -224,7 +223,7 @@ class Project:
     def module_working_directory(self, module_name):
         """
         Returns a working directory for the module
-        If the directory doesn't exist, the directory is created.
+        The directory is created if the directory doesn't exist.
 
         :param module_name: name for the module
         :returns: working directory
@@ -335,7 +334,7 @@ class Project:
         """
         Closes the project, and cleanup the disk if cleanup is True
 
-        :param cleanup: If True drop the project directory
+        :param cleanup: Whether to delete the project directory
         """
 
         tasks = []
@@ -431,7 +430,7 @@ class Project:
     @asyncio.coroutine
     def list_files(self):
         """
-        :returns: Array of files in project without temporary files. The files are dictionnary {"path": "test.bin", "md5sum": "aaaaa"}
+        :returns: Array of files in project without temporary files. The files are dictionary {"path": "test.bin", "md5sum": "aaaaa"}
         """
 
         files = []
@@ -479,8 +478,7 @@ class Project:
         """
 
         z = zipstream.ZipFile()
-        # topdown allo to modify the list of directory in order to ignore
-        # directory
+        # topdown allows to modify the list of directory in order to ignore the  directory
         for root, dirs, files in os.walk(self._path, topdown=True):
             # Remove snapshots and capture
             if os.path.split(root)[-1:][0] == "project-files":
@@ -638,4 +636,4 @@ class Project:
                 shutil.move(path, dst)
 
         # Cleanup the project
-        shutil.rmtree(root)
+        shutil.rmtree(root, ignore_errors=True)

gns3server/compute/qemu/qcow2.py

@@ -26,7 +26,7 @@ class Qcow2Error(Exception):
 
 class Qcow2:
     """
-    Allow to parse a Qcow2 file
+    Allows to parse a Qcow2 file
     """
 
     def __init__(self, path):

gns3server/controller/__init__.py

@@ -31,7 +31,7 @@ log = logging.getLogger(__name__)
 
 
 class Controller:
-    """The controller manage multiple gns3 compute servers"""
+    """The controller is responsible to manage one or more compute servers"""
 
     def __init__(self):
         self._computes = {}
@@ -47,17 +47,14 @@ class Controller:
         """
         Save the controller configuration on disk
         """
-        data = {
-            "computes": [{
-                "host": c.host,
-                "port": c.port,
-                "protocol": c.protocol,
-                "user": c.user,
-                "password": c.password,
-                "compute_id": c.id
-            } for c in self._computes.values()],
-            "version": __version__
-        }
+        data = {"computes": [{"host": c.host,
+                              "port": c.port,
+                              "protocol": c.protocol,
+                              "user": c.user,
+                              "password": c.password,
+                              "compute_id": c.id
+                              } for c in self._computes.values()],
+                "version": __version__}
         os.makedirs(os.path.dirname(self._config_file), exist_ok=True)
         with open(self._config_file, 'w+') as f:
             json.dump(data, f, indent=4)
@@ -73,15 +70,15 @@ class Controller:
             with open(self._config_file) as f:
                 data = json.load(f)
         except OSError as e:
-            log.critical("Can not load %s: %s", self._config_file, str(e))
+            log.critical("Cannot load %s: %s", self._config_file, str(e))
             return
         for c in data["computes"]:
             compute_id = c.pop("compute_id")
             yield from self.add_compute(compute_id, **c)
 
-    def isEnabled(self):
+    def is_enabled(self):
         """
-        :returns: True if current instance is the controller
+        :returns: whether the current instance is the controller
         of our GNS3 infrastructure.
         """
         return Config.instance().get_section_config("Server").getboolean("controller")
@@ -89,9 +86,9 @@ class Controller:
     @asyncio.coroutine
     def add_compute(self, compute_id, **kwargs):
         """
-        Add a server to the dictionnary of computes controlled by GNS3
+        Add a server to the dictionary of compute servers controlled by this controller
 
-        :param compute_id: Id of the compute node
+        :param compute_id: Compute server identifier
         :param kwargs: See the documentation of Compute
         """
 
@@ -100,36 +97,35 @@ class Controller:
             return self._create_local_compute()
 
         if compute_id not in self._computes:
-            compute = Compute(compute_id=compute_id, controller=self, **kwargs)
-            self._computes[compute_id] = compute
+            compute_server = Compute(compute_id=compute_id, controller=self, **kwargs)
+            self._computes[compute_id] = compute_server
             self.save()
         return self._computes[compute_id]
 
     def _create_local_compute(self):
         """
-        Create the local compute node. It's the controller itself
+        Create the local compute node. It is the controller itself.
         """
         server_config = Config.instance().get_section_config("Server")
-        self._computes["local"] = Compute(
-            compute_id="local",
-            controller=self,
-            protocol=server_config.get("protocol", "http"),
-            host=server_config.get("host", "localhost"),
-            port=server_config.getint("port", 3080),
-            user=server_config.get("user", ""),
-            password=server_config.get("password", ""))
+        self._computes["local"] = Compute(compute_id="local",
+                                          controller=self,
+                                          protocol=server_config.get("protocol", "http"),
+                                          host=server_config.get("host", "localhost"),
+                                          port=server_config.getint("port", 3080),
+                                          user=server_config.get("user", ""),
+                                          password=server_config.get("password", ""))
         return self._computes["local"]
 
     @property
     def computes(self):
         """
-        :returns: The dictionnary of computes managed by GNS3
+        :returns: The dictionary of compute server managed by this controller
         """
         return self._computes
 
     def get_compute(self, compute_id):
         """
-        Return an compute or raise a 404
+        Returns a compute server or raise a 404 error.
         """
         try:
             return self._computes[compute_id]
@@ -141,21 +137,21 @@ class Controller:
     @asyncio.coroutine
     def add_project(self, project_id=None, **kwargs):
         """
-        Create a project or return an existing project
+        Creates a project or returns an existing project
 
         :param kwargs: See the documentation of Project
         """
         if project_id not in self._projects:
             project = Project(project_id=project_id, **kwargs)
             self._projects[project.id] = project
-            for compute in self._computes.values():
-                yield from project.add_compute(compute)
+            for compute_server in self._computes.values():
+                yield from project.add_compute(compute_server)
             return self._projects[project.id]
         return self._projects[project_id]
 
     def get_project(self, project_id):
         """
-        Return a project or raise a 404
+        Returns a compute server or raise a 404 error.
         """
         try:
             return self._projects[project_id]
@@ -168,7 +164,7 @@ class Controller:
     @property
     def projects(self):
         """
-        :returns: The dictionnary of computes managed by GNS3
+        :returns: The dictionary of computes managed by GNS3
         """
         return self._projects
 
@@ -176,6 +172,7 @@ class Controller:
     def instance():
         """
         Singleton to return only on instance of Controller.
+
         :returns: instance of Controller
         """
 
@@ -195,5 +192,5 @@ class Controller:
             except KeyError:
                 pass
         else:
-            for project in self._projects.values():
-                project.emit(action, event, **kwargs)
+            for project_instance in self._projects.values():
+                project_instance.emit(action, event, **kwargs)

gns3server/controller/compute.py

@@ -18,8 +18,8 @@
 import aiohttp
 import asyncio
 import json
-from pkg_resources import parse_version
 
+from ..utils import parse_version
 from ..controller.controller_error import ControllerError
 from ..config import Config
 from ..version import __version__

gns3server/controller/link.py

@@ -33,6 +33,7 @@ class Link:
         self._project = project
         self._capturing = False
         self._capture_file_name = None
+        self._streaming_pcap = None
 
     @asyncio.coroutine
     def add_node(self, node, adapter_number, port_number):
@@ -73,13 +74,12 @@ class Link:
     @asyncio.coroutine
     def _start_streaming_pcap(self):
         """
-        Dump the pcap file on disk
+        Dump a pcap file on disk
         """
         stream = yield from self.read_pcap_from_source()
         with open(self.capture_file_path, "wb+") as f:
             while self._capturing:
-                # We read 1 bytes by 1 otherwise if the traffic stop the remaining data is not read
-                # this is slow
+                # We read 1 bytes by 1 otherwise the remaining data is not read if the traffic stops
                 data = yield from stream.read(1)
                 if data:
                     f.write(data)
@@ -98,7 +98,7 @@ class Link:
     @asyncio.coroutine
     def read_pcap_from_source(self):
         """
-        Return a FileStream of the Pcap from the compute node
+        Return a FileStream of the Pcap from the compute server
         """
         raise NotImplementedError
 
@@ -106,13 +106,12 @@ class Link:
         """
         :returns: File name for a capture on this link
         """
-        capture_file_name = "{}_{}-{}_to_{}_{}-{}".format(
-            self._nodes[0]["node"].name,
-            self._nodes[0]["adapter_number"],
-            self._nodes[0]["port_number"],
-            self._nodes[1]["node"].name,
-            self._nodes[1]["adapter_number"],
-            self._nodes[1]["port_number"])
+        capture_file_name = "{}_{}-{}_to_{}_{}-{}".format(self._nodes[0]["node"].name,
+                                                          self._nodes[0]["adapter_number"],
+                                                          self._nodes[0]["port_number"],
+                                                          self._nodes[1]["node"].name,
+                                                          self._nodes[1]["adapter_number"],
+                                                          self._nodes[1]["port_number"])
         return re.sub("[^0-9A-Za-z_-]", "", capture_file_name) + ".pcap"
 
     @property

gns3server/controller/project.py

@@ -32,7 +32,7 @@ from ..utils.path import check_path_allowed, get_default_project_directory
 
 class Project:
     """
-    A project inside controller
+    A project inside a controller
 
     :param project_id: force project identifier (None by default auto generate an UUID)
     :param path: path of the project. (None use the standard directory)
@@ -89,7 +89,7 @@ class Project:
             raise aiohttp.web.HTTPInternalServerError(text="Could not create project directory: {}".format(e))
 
         if '"' in path:
-            raise aiohttp.web.HTTPForbidden(text="You are not allowed to use \" in the project directory path. It's not supported by Dynamips.")
+            raise aiohttp.web.HTTPForbidden(text="You are not allowed to use \" in the project directory path. Not supported by Dynamips.")
 
         self._path = path
 
@@ -201,7 +201,7 @@ class Project:
 
         :param action: Action name
         :param event: Event to send
-        :param kwargs: Add this meta to the notif (project_id for example)
+        :param kwargs: Add this meta to the notification (project_id for example)
         """
         for listener in self._listeners:
             listener.put_nowait((action, event, kwargs))

gns3server/controller/udp_link.py

@@ -108,8 +108,7 @@ class UDPLink(Link):
         """
         Run capture on the best candidate.
 
-        The ideal candidate is a node who support capture on controller
-        server
+        The ideal candidate is a node who support capture on controller server
 
         :returns: Node where the capture should run
         """

tests/controller/test_controller.py

@@ -69,9 +69,9 @@ def test_load(controller, controller_config_path, async_run):
 
 def test_isEnabled(controller):
     Config.instance().set("Server", "controller", False)
-    assert not controller.isEnabled()
+    assert not controller.is_enabled()
     Config.instance().set("Server", "controller", True)
-    assert controller.isEnabled()
+    assert controller.is_enabled()
 
 
 def test_addCompute(controller, controller_config_path, async_run):