Improve code quality - Githubissues

citelab / gini5

GINI Is Not Internet - A toolkit for constructing virtual micro Internet

https://citelab.github.io/gini5

MIT License

12 stars 8 forks source link

Improve code quality #48

Open trungams opened 5 years ago

trungams commented 5 years ago

It would be good if we can make Gini compatible with Python3, since support for version 2 will be dropped soon. Also, since the structure is quite complicated, we should improve the quality in terms of code commenting, modularization, following style guides, etc.

[ ] Modularize the software: This is related to #40 and #41. One idea is provide abstract classes for each network component so that it's easier to add different concrete implementation.
[ ] Add code commenting: This helps us understand how the GUI components interact and how front-end - back-end communicate. This makes debugging and improving routing/compiling algorithm possible.
[ ] Follow style guides: https://www.python.org/dev/peps/pep-0008/
[ ] Add tests and travis integration: There used to be TravisCI for Gini3: https://github.com/anrl/gini3. We can adapt from the old test suite and add more cases

trungams commented 5 years ago

Regarding modularization, the models on the backend and frontend are currently inconsistent, and I also think it's a good idea to separate components into 2(+) categories: host elements (end devices) and network elements (switch/routing devices)

trungams commented 5 years ago

When building Gini using Scons, it's better if we structure each component the similar to the source code, as in it should be split into lib/frontend and lib/backend. This makes adding/removing python packages easier

E.g: a python file can import another module using from backend import ... instead of using relative module path

trungams commented 5 years ago

Rely on SDK instead of subprocess calls to manage docker (and ovs). Pox is already good to use

Docker SDK for Python: https://github.com/docker/docker-py
ovs Python package: https://pypi.org/project/ovs/. They don't document this so it's rather unofficial or not really what we're using ovs for? We can have a look at this file however for some idea on how to bind ovs calls, and subprocess calls ourselves in general

trungams commented 5 years ago

One thing to consider is to run gbuilder (and consequently, other components) as root, so we don't need to change a lot of the configurations (setuid bits, wiresharks, docker, etc) during installation.

One obvious advantage of this is we can now run ovs-docker without having to create a netns directory and change ownership first. Since it's a bash script instead of a binary executable, setuid bit has zero effect.

https://unix.stackexchange.com/questions/364/allow-setuid-on-shell-scripts