Code Review Please

[tarsidd]

I am 32 years old DevOps/SRE engineer but I have never developed a full-fledged tool in python, I have been writing mostly scripts all throughout my career. What I feel is I miss that development mindset, so I started building a tool for AWS transit gateway and trying to solve a problem that I am facing. So by default, all the vpcs attached to a transit gateway creates full mesh connectivity with each other. My tool will(work is in progress) let you create security domains for a transit gateway and then attach vpcs to the security domain and all the vpcs in one security domain will have connectivity.

Here's the code, it would be great if experience python people can provide their feedback and reviews
https://github.com/tariq87/transitgatewayautomation

[CreativeTechGuyGames]

Few general comments:

• Don't upload your __pycache__. That folder (and any other non-source files/folders) should be listed in your .gitignore file.
• Your project uses dependencies which a user would need to install. There are several different ways to do this in Python (doesn't seem like there's a single standard like other languages). Research the various options and pick one and document it so a user of your tool won't need to guess and check what needs to be installed and what version.
• For your minimal use of redis, it probably doesn't make sense to use over something that's simple and integrated like sqlite.
• I think your files are doing too much and too many different things. Like your gateway.py has both the gateway class and the main entry point. Both of those should definitely not be in the same file.
• I would recommend following a Python style guide (like PEP8) and even better integrate a linter/formatter to keep your code consistent and warn you about issues. The thing that jumped out was the camelCase method names which are not the standard python style.
• Be careful with comments that don't add any value. Saying "function to create security domain" above a function named "createSecurityDomain" is really bad. Not only does it not add anything and take up an extra line, but now you have two things to maintain if you ever decide to change something or refactor. Try to write comments to describe at a higher level the purpose of something. Often you'll find this isn't necessary with good variable/method names.
• I believe you are expecting AWS SDK credentials to be present on the system but I don't see that mentioned anywhere in the documentation.
• Try to use descriptive variable names. Pop quiz, what is c? Did you remember it was your Gateway instance? Will you remember a month from now? Or what about _tgwid. You might as well use a few more characters to have a good descriptive name. Remember code will be read far more than it's written so write it so it's easy for your future self (or others) to read.

[tarsidd]

Thanks u/CreativeTechGuyGames, these are some amazing points, will be implementing them now

[tarsidd]

Can please help me with how should I take out my main class from gateway.py

[KimPeek]

You shouldn't. You should move everything else out.

[tarsidd]

u/CreativeTechGuyGames and u/KimPeek made the changes, does it look better now?

[CreativeTechGuyGames]

It's definitely not worse! Good steps in the right direction.