On the prompting of Jason Myers, I'm going to try my best to keep a journal of little code puzzles and improvements that I make in the course of writing and maintaining open source code in my never ending search for higher quality code.
Recently, I've been doing a lot of work prepping a relaunch of django-stripe-payments, renamed along the way to pinax-stripe.
In this process, I have gone down the rabbit hole of trying all kinds of static analysis things to help inform and drive quality up.
A recent thing that Jason brought to my attention today was bandit. So without delay, I installed and ran it with defaults against pinax-stripe. It reported three issues that previous static analyzers failed to point out:
Run metrics:
Total lines of code: 2547
Total lines skipped (#nosec): 0
Total issues (by severity):
Undefined: 0.0
Low: 3.0
Medium: 0.0
High: 0.0
Total issues (by confidence):
Undefined: 0.0
Low: 0.0
Medium: 0.0
High: 3.0
All three issues were cases of using pass in a try/except block.
The full details of my fixes can be found in this commit.
I'd like to point the fixes individually though as well:
The first example was just a case of reversing the logic so that I only re-raise the exception if it's not the thing we want to ignore. No wizardry there.
The next fix is probably one of my favorite tricks. Often you want to effectively retrieve a single object or return None. It's ok if it doesn't exist and if multiple exist you just need the first one. You can either write a queryset check the count or exists before indexing into it, or wrap a get in a try/except (which is what I originally had in this case), or you can use next and iter do things in a single line and single query. In this example, it was:
customer = next(iter(queryset)), None)
Then you can check if the customer is None and act on it accordingly.
The third fix was simply checking for the existence of the field name in a way that wouldn't raise an exception. In this specific case, I want to add the ability to search by a user's email if the user model in question has an email field like the default django.contrib.auth.User has.
In the fourth and last fix I simply needed to use the .get() method on the response object then I could avoid needing to worry about the KeyError all together.
Always run git clean -fdx prior to creating a Python release to eliminate possibility of unknown bits getting including in your package.
Discovered a file I had deleted from a previous release laying around in a build/ directory that was getting shipped with subsequent releases somehow, but only in the wheel releases, not the standard sdist.
In Django forms, you can either use exclude or fields to list a set of fields to exclude or include on the ModelForm.
I prefer to use fields to explicitly include fields on a form because it's easy to add fields to a model without thinking through it's use on a form somewhere in your project. However, if you are adding a field for the purpose of adding it to a form you'll have the form in mind so explicitly adding the field to the fields list will not be forgotten about.