Hello Michael,
a big thanks for your feedback, this is my first own project. I also imagined that there will be a couple of problems, and a lot of stuff will be missing or wrong.
So I'm going to solve the reported issues, step by step and report my changes.
Best regards,
Stefan
Okay, here is the review of the tool:
- No build system.
The tool is very poorly packaged. It is not easy to distinguish between frontend and backend. The database.py file is not properly encoded for Linux.
You should start using the python setuptools and create a backend module and frontend tools.
- Database format is missing.
There is no indication what the database should look like. Which leads in general to 3). Also, I was not able to run the code.
- No documentation at all.
Man pages are mandatory. A good documentation how to use the module is important for developers. There should also be some description about how the code works internally (which is not that important here).
- In cli.py (line 29), the backend gets always initialized. This leaves
no chance to load a different database file (it's sqlite) and is completely unnecessary in case you just want to see the help (cli.py --help).
- Parsing command line arguments is fine. That was copied from pakfire
anyway. Handling the given commands is less elegant. The functions (e.g. handle_show_records) are too long and could be broken down. These nested conditions make it hard to keep track.
Why is there SOARecord, ARecord and Record?
Everywhere, where the database is accessed it is not checked whether
valid content is returned or just None. This will cause crashes of the entire application for invalid requests.
- No caching is used. Every single attribute has to be fetched several
times from the database which may cause a performance penalty. As the length of the records is pretty small, I'd say it can be fetched all at once. Same for domains.
- Nearly all coding guidelines of the project have been totally
ignored. Especially that the limit of line length is 80 characters (soft). Wrong usage of the doc strings. Wrong usage of global variables. Unused code is commented instead of being removed.
- The translation which has been started in i18n.py is completely
missing.
- In i18n.py, the project is called "pdns-tool". In cli.py, it is
called "pdns". The git repository is called pydns. The name in the description is pyPDNS. This is confusing.
- There should be unit tests.
That's all from me for now.
Michael
On Sun, 2012-09-30 at 20:01 +0200, Stefan Schantl wrote:
Hello,
today I want to introduce you to pyPDNS.
I is designed as an administration tool for the authoritative DNS server from PowerDNS, and is completely written in Python. When development has been finished, it's goal will be to get, add / remove or to edit zones and their records directly on the PDNS sqlite3 database backend.
It will be possible to use it as a command line interface or by import it into other python applications like the web interface of IPFire 3.
I've just created the git repository and pushed it on our git server, so the development process is an an very early state and a lot of important functions are still missing. At the moment it is only possible to use the API and the CLI to get the configured zones and records.
If you are interested on these tool, you can get the source code here:
http://git.ipfire.org/?p=people/stevee/pypdns.git;a=summary
Development mailing list Development@lists.ipfire.org http://lists.ipfire.org/mailman/listinfo/development
Development mailing list Development@lists.ipfire.org http://lists.ipfire.org/mailman/listinfo/development