X

Aggregator

CodeSOD: In the Know

The Daily WTF
1 month 3 weeks ago

Delilah works in a Python shop. Despite Python's "batteries included" design, that doesn't stop people from trying to make their own batteries from potatoes. For example, her co-worker wrote this function:

def key_exists(element, key): if isinstance(element, dict): try: element = element[key] except KeyError: return False return True

Python, of course, has an in operator. key in dictionary is an extremely common idiom. There's no reason to implement your own. Certainly, there's no reason to re-implement it by catching and throwing exceptions.

This is ugly, stupid, and bad. It gets worse, though, when you see how it gets used.

for key in old_yaml_data: if key in new_yaml_data: if old_yaml_data[key] != new_yaml_data[key]: temp = new_yaml_data[key] new_yaml_data[key] = merge(new_yaml_data[key], old_yaml_data[key]) if key_exists(new_yaml_data[key], 'image') and key_exists(old_yaml_data[key], 'image'): new_yaml_data[key]['image'] = temp['image'] elif key == "databases": revert_db_tags(new_yaml_data[key], temp)

This code is attempting to upgrade "old" YAML data with "new" data. So it's basically merging dictionaries, which is a great case for the in operator.

And they use the correct idiom on the second line there! This was written by one developer! They do the standard key in new_yaml_data check. And they also use key_exists. I can only assume that they had a stroke between starting and finishing this script, which I'll note is, in total, 48 lines long.

Here's the whole short script, which is just generally a mess. Slapped together Python code that's trying to be a "smarter" shell script, but is definitely written with the elegance of hacked-together-bash.

import sys import yaml from jsonmerge import merge appHomePath = sys.argv[1] oldValuesYAML = appHomePath + "values.yaml" newValuesYAML = appHomePath + "/upgrade_version/values.yaml" with open(newValuesYAML, 'r') as f: new_yaml_data = yaml.load(f, Loader=yaml.loader.FullLoader) with open(oldValuesYAML, 'r') as f: old_yaml_data = yaml.load(f, Loader=yaml.loader.FullLoader) def key_exists(element, key): if isinstance(element, dict): try: element = element[key] except KeyError: return False return True def revert_db_tags(old_yaml_data, new_yaml_data): dbList = ["mongoDB", "postgresDB"] mongoDbTagsToRevert = ["mongoRestore"] mongodbKeysToDelete = [] postgresDbTagsToRevert = [] for db in dbList: old_yaml_data[db]['image'] = new_yaml_data[db]['image'] for mongoDbTag in mongoDbTagsToRevert: old_yaml_data['mongoDB'][mongoDbTag]['image'] = new_yaml_data['mongoDB'][mongoDbTag]['image'] for mongoDbTag in mongoKeysToDelete: del old_yaml_data['mongoDB'][mongoDbTag] for postgresDbTag in postgresDbTagsToRevert: old_yaml_data['postgresDB'][postgresDbTag]['image'] = new_yaml_data['postgresDB'][postgresDbTag]['image'] for key in old_yaml_data: if key in new_yaml_data: if old_yaml_data[key] != new_yaml_data[key]: temp = new_yaml_data[key] new_yaml_data[key] = merge(new_yaml_data[key], old_yaml_data[key]) if key_exists(new_yaml_data[key], 'image') and key_exists(old_yaml_data[key], 'image'): new_yaml_data[key]['image'] = temp['image'] elif key == "databases": revert_db_tags(new_yaml_data[key], temp) with open(newValuesYAML, 'w') as f: data = yaml.dump(new_yaml_data, f, sort_keys=False) [Advertisement] Plan Your .NET 9 Migration with Confidence
Your journey to .NET 9 is more than just one decision.Avoid migration migraines with the advice in this free guide. Download Free Guide Now!
Remy Porter