Python Forum

I hope it's ok to ask for what may be basic help. I am a hobbyist and newish to Python, but used to code in COBOL many years ago. I'd like some help on how you would re-write the structure of my code.

I like to create code to protect my home network via my router and have written some code (below) that successfully reads and processes a file from a website. The file is a list of IP addresses of tor exit points. I can only download it once a day from the remote website so need to save it the first time so I can run the script more than once if I need to. The script has been running successfully for many months.

The code works but I recognise that is it completely inelegant. It drops in and out of if statement and doesn't save the file until the end. I'm hoping someone could give me a bit of help to structure the code in a more elegant pythonesque way. Should I be using Try statements to catch the != 200, but how big should the try statement be?

I realise this is only a snippet, but a response giving some structured pseudocode that I can copy would be very helpful.

TIA, Charles

##############################################################################
# Get tor file once a day from the internet and save it for another run
##############################################################################
print('>>tor')
count = 0
AddressFile = datePrefix + '-torlist.txt'

if os.path.isfile('html/' + AddressFile):
url = 'http://192.168.xxx.xxx/' + AddressFile
print('>>tor from file')
else:
url = 'https://www.xxxxxxxxx/torlist/?exit'
print('>>tor from web')
response = requests.get(url)
if response.status_code != 200:
print('Failed to get data:', response.status_code)
else:
wrapper = csv.reader(response.text.strip().split('\n'), delimiter=';')
for record in wrapper:
if record[0][0:1].isnumeric() and len(record[0]) != 39:
OutText = record[0] + "/32"
ip_list.append(IPNetwork(OutText))
count += 1
print('>>tor: ' + str(count))

if not os.path.isfile('html/' + AddressFile):
f = open('html/' + AddressFile, "w")
f.write(str(response.text))
f.close()

This is just a fragment of your code; However, I would refactor as follows:
1) use logging instead of printing;
2) Lines No. 8-13: return url? these lines could be pushed to a function, e.g. def get_url(...):
3) requests.get -- may lead to an exception (e.g. if url is bad); you probably need to wrap it with try/except;
4) you don't need to wrap status_code checking with try/except (answer to your question);
5) csv.reader can raise an exception, if loaded content is not csv-compliant; probably, you need to put try/except here. Anyway, I would put these lines into some function.
6) Use with when opening files, e.g. with open(os.path.join('html', AddressFile) as out_file: (in this case you don't need to care about closing the file (out_file.close() isn't needed anymore))
7) Another option is to use pathlib when you need concatenate paths.

One idea which might be worth of considering is to use 'x' (create) mode while opening the file. This will raise FileExistsError if file is already existing.

try:
    with open(filename, 'xt') as f:   # t for text mode, b for binary mode
        # do stuff
except FileExistsError:
    # do other stuff       

Also you may consider pepify/prettify the code using variable naming convetions and f-strings (address_file = f'{date_prefix}-torlist.txt')

Beware of .isnumeric expected behaviour as well:

>>> digits = '123'
>>> numerics = '一二三'   # '123'in Kanji
>>> decimals = '1²'
>>> digits.isnumeric()
True
>>> numerics.isnumeric()
True
>>> decimals.isnumeric()
True