1
\$\begingroup\$

This is my first attempt at using Python to send http requests. I want to keep a zone record on cPanel pointed at my home network's public IP. I'm just looking for some general feedback/suggestions. Is there an easier way to do this? Is there a more Pythonic way? My intent is to run this as a cronjob to stay ahead of my ISP's DHCP.

This file generates the http request containing the cPanel JSON API call.

#!/usr/bin/python3

from http import client
from base64 import b64encode
from sys import argv
from netutils import PollIP

class ZoneUpdate:

    def __init__ (self):

        pass

    def updateIP (self, passedIP):

        newIP = passedIP 

        conn = client.HTTPSConnection('mydomain.com', 2083)

        myAuth = b64encode(b'username:thepassword').decode('ascii')

        authHeader = {'Authorization':'Basic ' + myAuth}

        conn.request('GET', '/json-api/cpanel?cpanel_jsonapi_version=2&cpanel_jsonapi_module=ZoneEdit&cpanel_jsonapi_func=edit_zone_record&domain=mydomain.com&line=28&class=IN&type=A&name=homenet.mydomain.com.&ttl=3600&address=' + newIP, headers=authHeader)

        myResponse = conn.getresponse()

        print(myResponse.getcode())

        data = myResponse.read()

        if myResponse.getcode() != 200:
        print('did not succeed')

        print (data)

############################################

if __name__ == "__main__":

    app = ZoneUpdate()

    if len(argv) > 1:
        newIP = argv[1]
        print(argv[1])
    else:    
        getIP = PollIP()
        newIP = getIP.publicIP()

    app.updateIP(newIP)

This module (imported by the preceding) will use the dyndns website to obtain my public IP address.

#!/usr/bin/python3

from urllib import request
from re import compile

class PollIP:

    def __init__(self):
        pass

    #Use the dyndns web-site to poll my public IP address 
    def publicIP(self):

        myReq = request.urlopen('http://checkip.dyndns.com')

        html = str(myReq.read())

        myReg = compile('[\d.]+')

        myMatch = myReg.search(html)

        print(myMatch.group())

        return myMatch.group()
\$\endgroup\$
1
\$\begingroup\$

Before anything else, I'd like to point out that there is probably an error with your indentation. I've fixed it in my code assuming that only one line was not indented properly..

Simple is better than complex.

  • You don't need the classes (you can also have a look at this video). After taking this simple comment into account, your code looks like this (I put everything in the same place for the sake of simplicity)
#!/usr/bin/python3

from http import client
from base64 import b64encode
from sys import argv
from netutils import PollIP
from urllib import request
from re import compile

#Use the dyndns web-site to poll my public IP address 
def publicIP():
    myReq = request.urlopen('http://checkip.dyndns.com')
    html = str(myReq.read())
    myReg = compile('[\d.]+')
    myMatch = myReg.search(html)
    print(myMatch.group())
    return myMatch.group()

def updateIP (passedIP):
    newIP = passedIP 
    conn = client.HTTPSConnection('mydomain.com', 2083)
    myAuth = b64encode(b'username:thepassword').decode('ascii')
    authHeader = {'Authorization':'Basic ' + myAuth}
    conn.request('GET', '/json-api/cpanel?cpanel_jsonapi_version=2&cpanel_jsonapi_module=ZoneEdit&cpanel_jsonapi_func=edit_zone_record&domain=mydomain.com&line=28&class=IN&type=A&name=homenet.mydomain.com.&ttl=3600&address=' + newIP, headers=authHeader)
    myResponse = conn.getresponse()
    print(myResponse.getcode())
    data = myResponse.read()
    if myResponse.getcode() != 200:
        print('did not succeed')
    print (data)

############################################

if __name__ == "__main__":
    if len(argv) > 1:
        newIP = argv[1]
        print(argv[1])
    else:    
        newIP = publicIP()
    updateIP(newIP)
  • Your variables names are probably not that good. What do you mean by 'new' ? Also, many variables are not even required (I've been pretty far in the cleanup as it helped me to understand the code, you might want to keep some of the variables I've removed). Also, your function names are not really well chosen as it doesn't really describe what they do : updateIP does not seem to update the IP at all.

  • Separate the concerns : do you want publicIP() to return a string or to print it ? It should probably just return it and then you print it from the main if you want. Please note that doing this introduces an interesting pattern : you print newIP from the 2 branches of your if. You can easily get rid of this duplicated code. In the same spirit, updateIP(ip) seems to be printing things and does not return anything. Let's name the function to make this easier to understand. Please note that : 1) you'd probably want this to return a string in a real life situation 2) you might want it to throw an exception if something fails 3) my functions names are terrible 4) there are naming convention for Python that you'll find in PEP 008 (it's worth a read).

  • You can get rid of values (urls, ports, login, password, etc) in the middle of the code. This can be useful if you need to reuse them in different places. In any case, it's worth replacing 200 by client.OK which is much clearer.

Once you've done so, here what you code is like :

#!/usr/bin/python3

from http import client
from base64 import b64encode
from sys import argv
from netutils import PollIP
from urllib import request
from re import compile

#Use the dyndns web-site to poll my public IP address 
def getPublicIP():
    html = str(request.urlopen('http://checkip.dyndns.com').read())
    reg = compile('[\d.]+')
    match = reg.search(html)
    return match.group()

def printIPFromIP(ip):
    conn = client.HTTPSConnection('mydomain.com', 2083)
    conn.request('GET', '/json-api/cpanel?cpanel_jsonapi_version=2&cpanel_jsonapi_module=ZoneEdit&cpanel_jsonapi_func=edit_zone_record&domain=mydomain.com&line=28&class=IN&type=A&name=homenet.mydomain.com.&ttl=3600&address=' + ip, 
        headers={'Authorization':'Basic ' + b64encode(b'username:thepassword').decode('ascii')})
    response = conn.getresponse()
    print(response.getcode())
    if response.getcode() != client.OK:
        print('did not succeed')
    print (response.read())

############################################

if __name__ == "__main__":
    ip = argv[1] if len(argv) > 1 else getPublicIP()
    print(ip)
    printIPFromIP(ip)

It's probably not perfect but I think I hilighted the main issues.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for the thorough response and helpful resources! \$\endgroup\$
    – Joseph
    Apr 9 '13 at 18:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.