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: > > 1) 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. > > 2) 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. > > 3) 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). > > 4) 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). > > 5) 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. > > 6) Why is there SOARecord, ARecord and Record? > > 7) 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. > > 8) 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. > > 9) 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. > > 10) The translation which has been started in i18n.py is completely > missing. > > 11) 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. > > 12) 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(a)lists.ipfire.org >> http://lists.ipfire.org/mailman/listinfo/development > _______________________________________________ > Development mailing list > Development(a)lists.ipfire.org > http://lists.ipfire.org/mailman/listinfo/development >