Re: New tool - pyPDNS

[Stefan Schantl <stefan.schantl@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 3969 bytes --]

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
>