Open cstephen opened 2 years ago
Another comment to sharpen this. Here's an example from another code path:
# Before (code as it exists now):
# boundary.py ~l77
poly_type = validate_var_id(var_id)
if type(poly_type) is tuple:
return poly_type
try:
poly = type_di[poly_type].loc[[var_id]].to_crs(4326)
except:
return render_template("422/invalid_area.html"), 422
# validate_request.py ~l63
def validate_var_id(var_id):
if re.search("[^A-Za-z0-9]", var_id):
return render_template("400/bad_request.html"), 400
for filename in os.listdir("data/jsons/"):
with open(os.path.join("data/jsons/", filename), "r") as f:
for jsonline in json.loads(f.read()):
if var_id == jsonline["id"]:
return jsonline["type"]
return render_template("422/invalid_area.html"), 400
What could be done differently here would be to use exceptions so that the calling code can be written like this:
# boundary.py
try:
poly_type = validate_var_id(var_id)
poly = type_di[poly_type].loc[[var_id]].to_crs(4326)
except InvalidRequestError: ...
except UnknownAreaIdError: ...
For many of the endpoints, we have made a first pass at handling exceptions by overgeneralizing. In the
boundary.py
endpoint, for example:Failing to support the provided protected area ID is just one of the things that can go wrong here.
We need to take an inventory of the try/except blocks that are overgeneralizing and deal with more specific things that can go wrong, and possible make more HTTP error templates with more precise language to handle each case.