1
\$\begingroup\$

Recently I made a script to get available IP automatically. I was woundering if anyone can give me tips on how to make this code look better, preferably in a class and OOP. I'm gonna show the code to my boss, but i want it to look clean and nice before i do so, and hopefully learn a thing or two about writing better code.

Code:

import requests
from orionsdk import SwisClient
import getpass

# Brukerinfromasjon
npm_server = 'mw-solarwinds.yy.dd'
username = 'jnk'
password = getpass.getpass()

server_navn = input('Skriv inn DNS navn: ')
dns_ip = '10.96.17.4'  # 10.96.17.5 = Felles
dns_sone = 'yy.dd'

verify = False

if not verify:
    from requests.packages.urllib3.exceptions import InsecureRequestWarning

    requests.packages.urllib3.disable_warnings(InsecureRequestWarning)

swis = SwisClient(npm_server, username, password)  # Kjører mot IPAM med brukerinformasjon

subnets = {
    'ka-windows': ['10.112.12.0', '24'],
    'ka-linux': ['10.112.10.0', '24'],
    'ka-exa-mgmt': ['10.112.26.0', '28']
}

print("Tilgjengelige subnets: ")

for i in subnets:
    print(i)

print("--------------")

found = False

while not found:
    inp = input("Skriv in Subnet: ")
    if inp in subnets:
        '''                                                                                                   
        Finner ledig IP adresse i subnet                                                                      
        '''

        sub_ip = subnets[inp][0]
        sub_cdir = subnets[inp][1]
        ipaddr = swis.invoke('IPAM.SubnetManagement', 'GetFirstAvailableIp', sub_ip, sub_cdir)
        '''                                                                                                   
        Sette DNS > IP                                                                                        
        '''
        dns = swis.invoke('IPAM.IPAddressManagement', 'AddDnsARecord', server_navn, ipaddr, dns_ip, dns_sone)
        print("IP: {} > DNS: {}".format(ipaddr, server_navn))
        found = True
    else:
        print("Det er ikke et subnet, velg en fra listen.")
\$\endgroup\$
  • 1
    \$\begingroup\$ Your title is too generic. Your title should describe what you r code does, not what you want out of the review. \$\endgroup\$ – BCdotWEB May 16 at 13:56
  • 1
    \$\begingroup\$ It might also help to translate console output from your native language to English before posting it here for review. \$\endgroup\$ – AlexV May 16 at 14:30
2
\$\begingroup\$

Not sure if it's any better as a class...

#!/usr/bin/env python


import getpass
import requests

from orionsdk import SwisClient
from requests.packages.urllib3.exceptions import InsecureRequestWarning


class Subnet_Explorer(dict):
    def __init__(self, npm_server, auth, dns, verify, **kwargs):
        super(Subnet_Explorer, self).__init__(**kwargs)
        self.update(
            npm_server = npm_server,
            auth = auth,
            dns = dns,
            server_navn = input('Skriv inn DNS navn: '),
            swis = SwisClient(npm_server, auth['username'], auth['password'])
        )

        if verify == False:
            requests.packages.urllib3.disable_warnings(InsecureRequestWarning)

    def directed_exploration(self, subnets):
        """
        Yields tuple of IP and DNS addresses from select subnets
        """
        unexplored_subnets = subnets.keys()
        while True:
            print("Unexplored Subnets: {unexplored_subnets}".format(
                unexplored_subnets = unexplored_subnets))
            inp = input("Skriv in Subnet: ")

            if not unexplored_subnets or inp not in unexplored_subnets:
                print("Det er ikke et subnet, velg en fra listen.")
                break

            unexplored_subnets.remove(inp)
            ipaddr = self['swis'].invoke('IPAM.SubnetManagement',
                                      'GetFirstAvailableIp',
                                      subnets[inp][0],
                                      subnets[inp][1])
            dns = self['swis'].invoke('IPAM.IPAddressManagement',
                                   'AddDnsARecord',
                                   self['server_navn'],
                                   ipaddr,
                                   self['dns']['ip'],
                                   self['dns']['sone'])

            yield ipaddr, self['server_navn']


if __name__ == '__main__':
    """
    Running as a script within this block, eg. someone ran;

        python script_name.py --args

    to get here, usually.
    """

    auth = {
        'username': 'jnk',
        'password': getpass.getpass(),
    }

    dns = {
        'ip': '10.96.17.4',
        'sone': 'yy.dd',
    }

    subnet_explorer = Subnet_Explorer(
        npm_server = 'mw-solarwinds.yy.dd',
        auth = auth,
        dns = dns,
        verify = False)

    exploration = subnet_explorer.directed_exploration(
        subnets = {
            'ka-windows': ['10.112.12.0', '24'],
            'ka-linux': ['10.112.10.0', '24'],
            'ka-exa-mgmt': ['10.112.26.0', '28']
        })

    print("--------------")
    for ipaddr, server_navn in exploration:
        print("IP: {ipaddr} > DNS: {server_navn}".format(
            ipaddr = ipaddr,
            server_navn = server_navn))
    print("--------------")

... though perhaps ya see some things ya like. Regardless ya may want to consider is adding some argparse stuff after the if __name__ == '__main__': line. And probably best to use anything from above modifications with care as I may have made things a bit more messy by turning towards classes.

Is there a reason for setting dns = swis.invoke(...) when it's not being used for anything?

\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you for this, im trying to learn writing more in classes. DNS variable that is set was used for debugging purposes, when printed it tells me operatilm was successfull. \$\endgroup\$ – Johnny Niklasson May 18 at 6:18
  • \$\begingroup\$ Most welcome @JohnnyNiklasson, ya may want to hunt down some of my other Python answers then, scattered about but there's nifty tricks also within the comments of some... If you're not yielding or returning the dns variable then might be a good idea to have an if self.get('debug', 0) >= 1: line just previous; in other-words avoid the work when debugging isn't required... Oh and Git (or some version) tracking is a good idea, also allows for generating metrics for the boss later ;-) \$\endgroup\$ – S0AndS0 May 18 at 17:25
  • \$\begingroup\$ Thanks for advices! :D \$\endgroup\$ – Johnny Niklasson May 19 at 18:01

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.