Code critique xmlrpclib

[flagg]

This xmlrpc server is designed to parse dns zone files and then
perform various actions on said files. \
It uses dnspython, and xmlrpclib
I'd like to know what some of the more experienced python users
think. Where I could improve code, make it more efficient, whatever.
All suggestions are welcome. This is my first functional python
program, and I have tons of questions to go along with whatever
suggestions. How could I do sanity checking; right now nothing
checks the input to these functions, error-handling in this program is
almost non-existant how do you integrate decent error handling into a
piece of software......ill shut up now.

Thanks


import dns.zone
from time import localtime, strftime, time
import os, sys
from dns.rdataclass import *
from dns.rdatatype import *
from string import Template
import xmlrpclib
from SimpleXMLRPCServer import SimpleXMLRPCServer
from SimpleXMLRPCServer import SimpleXMLRPCRequestHandler


zoneDir = "/dns"

def openZoneFile(zoneFile):
"""
Opens a zone file. Then reads in zone file to dnspython zone
object.
"""
global zone
global host
global domain
domain = ".".join(zoneFile.split('.')[1:])
host = ".".join(zoneFile.split('.')[:1])

try:
zone = dns.zone.from_file(zoneDir + domain + '.zone',
domain)
except dns.exception, e:
print e.__class__, e

class DNSFunctions:

# Not exposed to xml-rpc calls, used internally only.
def _writeToZoneFile(self, record):
"""
Checks if zone file exists, if it does it write values to zone
file
"""

for (name, ttl, rdata) in zone.iterate_rdatas(SOA):
new_serial = int(strftime('%Y%m%d00', localtime(time())))
if new_serial <= rdata.serial:
new_serial = rdata.serial + 1
rdata.serial = new_serial

if os.path.exists(zoneDir + str(zone.origin) + "zone"):
f = open(zoneDir + str(zone.origin) + "zone", "w")
zone.to_file(f)
f.close()
else:
print "Zone: " + zone.origin + " does not exist, please
add first"

def showRecord(self, record):
"""
Shows a record for a given zone. Prints out
TTL, IN, Record Type, IP
"""

openZoneFile(record)

try:
rdataset = zone.get_node(host)
for rdata in rdataset:
return str(rdata)
except:
raise Exception("Record not found")


def changeRecord(self, record, type, target):
"""
Changes a dns entry.
@param record: which record to chance
@param type: what type of record, A, MX, NS, CNAME
@target: if CNAME target points to HOSTNAME, if A record
points to IP
"""
openZoneFile(record)
try:
rdataset = zone.find_rdataset(host, rdtype=type)
except:
raise Exception("You must enter a valid record and type.
See --usage for examples")

for rdata in rdataset:
if rdata.rdtype == CNAME:
rdata.target = dns.name.Name((target,))
elif rdata.rdtype == A:
rdata.address = target
else:
assert False

self._writeToZoneFile(host)


def deleteRecord(self, record):
"""
Delete Entry from zone file
"""
openZoneFile(record)
zone.delete_node(host)
self._writeToZoneFile(record)

def addRecord(self, record, type, target):
"""
Adds a new record to zone file
"""
openZoneFile(record)

rdataset = zone.find_rdataset(host, rdtype=type, create=True)
if rdataset.rdtype == A:
rdata = dns.rdtypes.IN.A.A(IN, A, address = target)
rdataset.add(rdata, ttl=300)
elif rdataset.rdtype == CNAME:
rdata = dns.rdtypes.ANY.CNAME.CNAME(IN, CNAME,
dns.name.Name((target,)))
rdataset.add(rdata, ttl=300)
else:
assert False

self._writeToZoneFile(host)


def addZoneFile(self, file):
"""
Function to create a new empty zone file, if none exists. If
exists
function will terminate.
"""
newZoneSerial = strftime("%Y%m%d00", localtime())
zoneTemplate = Template('''$$TTL ${ttltime}
$$ORIGIN ${domain}.
@ 5M IN SOA nsrepo1.foobar.priv.
dnsadmin.foobar.priv. (
; did you update reverse?!
${serial} ; Serial CCYYMMDDxx where xx is
todays edit
1H
10M
1D
60M
)''')

if not os.path.exists(zoneDir + file + ".zone"):
f = open(zoneDir + file + ".zone", "w")
f.write(zoneTemplate.substitute({'ttltime':'300',
'domain': file, 'serial': newZoneSerial}))
f.close()
else:
print "Zone: " + file + " already exists"



class RequestHandler(SimpleXMLRPCRequestHandler):
# rpc_paths = ('/RPC2',),
pass

server = SimpleXMLRPCServer(("localhost", 8080), requestHandler =
RequestHandler, allow_none=True)

print "Listening on port 8080..."

server.register_introspection_functions()
server.register_instance(DNSFunctions())
server.register_multicall_functions()
server.serve_forever()


### Client Script ###
## I will probably use a mix of multicalls and singlecalls<?> for
client activity. Muticalls will primarily be used for showing a
record, applying a change, then showing the same record again to
verify changes went through.

import xmlrpclib

proxy = xmlrpclib.ServerProxy('http://localhost:8080')
multicall = xmlrpclib.MultiCall(proxy)



print proxy.showRecord('host11.lab0.foobar.priv')
print proxy.showRecord('host12.lab0.foobar.priv')

multicall.showRecord('host11.lab0.foobar.priv')
multicall.changeRecord('host11.lab0.foorbar.priv', 'A', '127.0.0.1')
multicall.showRecord('host11.lab0.foobar.priv')

print proxy.system.listMethods
result = multicall()

print result[1], result[2]

 

[flagg]

 This xmlrpc server is designed to parse dns zone files and then
 perform various actions on said files. \
 It uses dnspython, and xmlrpclib
  I'd like to know what some of the more experienced python users
 think. Where I could improve code, make it more efficient, whatever.
 All suggestions are welcome.  This is my first functional python
 program,  and I have tons of questions to go along with whatever
 suggestions.   How could I do sanity checking; right now nothing
 checks the input to these functions, error-handling in this program is
 almost non-existant how do you integrate decent error handling into a
 piece of software......ill shut up now.

I made a few comments, some about error handling!  See below

 import dns.zone
 from time import localtime, strftime, time
 import os, sys
 from dns.rdataclass import *
 from dns.rdatatype import *
 from string import Template
 import xmlrpclib
 from SimpleXMLRPCServer import SimpleXMLRPCServer
 from SimpleXMLRPCServer import SimpleXMLRPCRequestHandler

 zoneDir = "/dns"

 def openZoneFile(zoneFile):
     """
     Opens a zone file.  Then reads in zone file to dnspython zone
 object.
     """
     global zone
     global host
     global domain
     domain = ".".join(zoneFile.split('.')[1:])
     host = ".".join(zoneFile.split('.')[:1])

     try:
             zone = dns.zone.from_file(zoneDir + domain + '.zone',
 domain)
     except dns.exception, e:
             print e.__class__, e

Don't use global variables!

This function should probably return 3 things

    return zone, host, domain

And whenever you use it, say

    zone, host, domain = openZoneFile(xxx)

It should probably be a method of DNSFunctions.  I'd avoid setting
self.zone etc as that is making a different sort of global data which
I'd avoid too.

Also if the dns.zone.from_file didn't work (raises dns.exception) you
set the host and domain but the zone is left at its previous value
which probably isn't what you wanted.  I'd let the error propagate
which I think will do something sensible for the xmlrpc.

 class DNSFunctions:

      # Not exposed to xml-rpc calls, used internally only.
     def _writeToZoneFile(self, record):
         """
         Checks if zone file exists, if it does it write values to zone
 file
         """

         for (name, ttl, rdata) in zone.iterate_rdatas(SOA):
             new_serial = int(strftime('%Y%m%d00', localtime(time())))
             if new_serial <= rdata.serial:
                 new_serial = rdata.serial + 1
             rdata.serial = new_serial

         if os.path.exists(zoneDir + str(zone.origin) + "zone"):
             f = open(zoneDir + str(zone.origin) + "zone", "w")
             zone.to_file(f)
             f.close()
         else:
             print "Zone: " + zone.origin + " does not exist, please
 add first"

This should probably be raising an exception to be caught or
propagated back to the client via xmlrpc.

     def showRecord(self, record):
         """
         Shows a record for a given zone. Prints out
         TTL, IN, Record Type, IP
         """

         openZoneFile(record)

         try:
             rdataset = zone.get_node(host)
             for rdata in rdataset:
                 return str(rdata)
         except:
             raise Exception("Record not found")

     def changeRecord(self, record, type, target):
         """
         Changes a dns entry.
         @param record: which record to chance
         @param type:  what type of record, A, MX, NS, CNAME
         @target: if CNAME target points to HOSTNAME, if A record
 points to IP
         """
         openZoneFile(record)
         try:
             rdataset = zone.find_rdataset(host, rdtype=type)
         except:
             raise Exception("You must enter a valid record and type.
 See --usage for examples")

Don't catch all exceptions - find out which exceptions are thrown.
Consider just letting it propagate - hopefully the find_rdataset error
is descriptive enough.

Thanks for taking the time to reply I appreciate it. Are global
variables "bad"? Or just inefficient?

Also when you say:

"Also if the dns.zone.from_file didn't work (raises dns.exception) you
set the host and domain but the zone is left at its previous value
which probably isn't what you wanted. I'd let the error propagate
which I think will do something sensible for the xmlrpc."

Could you elaborate on that. Im not following what you mean by
"propagate"

Thanks again for the suggestions I'll remove the global variables and
use the method you described for assigning "host,domain and zone"