What happens
src/mock_vws/_services_validators/exceptions.py is 889 lines for 23 exception classes, and src/mock_vws/_query_validators/exceptions.py is 769 lines for 21. That is 1658 lines and 44 classes, averaging 38 lines each, and almost all of it is the same three steps repeated:
super().__init__()
self.status_code = HTTPStatus.NOT_FOUND
body = {
"transaction_id": uuid.uuid4().hex,
"result_code": ResultCodes.UNKNOWN_TARGET.value,
}
self.response_text = json_dump(body=body)
date = email.utils.formatdate(timeval=None, localtime=False, usegmt=True)
self.headers = {
"Connection": "keep-alive",
"Content-Type": "application/json",
"server": "envoy",
"Date": date,
"x-envoy-upstream-service-time": "5",
"Content-Length": str(object=len(self.response_text)),
"strict-transport-security": "max-age=31536000",
"x-aws-region": "us-east-2, us-west-2",
"x-content-type-options": "nosniff",
}
Grouping the classes by which headers they set:
_services_validators/exceptions.py: 23 classes
20 classes: Connection, Content-Length, application/json, x-aws-region
1 class: Connection, Content-Length, text/plain
1 class: Connection, Content-Length, text/html
1 class: Content-Length
_query_validators/exceptions.py: 21 classes
8 classes: Connection, Content-Length, application/json
4 classes: Connection, Content-Length
3 classes: Connection, Content-Length, WWW-Authenticate, text/plain
2 classes: Connection, Content-Length, text/plain
2 classes: Connection, Content-Length, WWW-Authenticate, application/json
1 class: Connection, Content-Length, text/html
1 class: Cache-Control, Connection, Content-Length, text/html
So 20 of the 23 services exceptions differ from each other in exactly two values: the status code and the result code. The query exceptions vary more, but still fall into a handful of shapes.
Why it matters
This is where header changes have to be made, and it is why they are expensive. #3365 opens by noting "There are 39 occurrences across src/" of the x-aws-region value, and most of those are here. Whatever is decided in that issue is a 39-site edit against the current structure and a one-line edit against a factored one. The same applies to server: envoy, x-envoy-upstream-service-time and strict-transport-security, which are equally hardcoded and equally likely to need revisiting.
The repetition also hides the interesting information. What is worth reading about UnknownTargetError is "404, UnknownTarget". That fact is currently surrounded by 30 lines which are identical to the previous class.
Suggested resolution
Move the response construction into the ValidatorError base class, so that the common case becomes a declaration:
class UnknownTargetError(ValidatorError):
"""..."""
status_code = HTTPStatus.NOT_FOUND
result_code = ResultCodes.UNKNOWN_TARGET
with the base building transaction_id, response_text and the standard header set, and hooks for the minority of classes which need a different content type, an extra header such as WWW-Authenticate, or a non-JSON body.
The two modules should keep their own base, since the services and query APIs genuinely have different standard header sets — the query API responses do not carry x-aws-region, for instance. Sharing the Date, Content-Length and content-type logic between the two bases is worthwhile; forcing one header set on both is not.
This is a pure refactor with no behaviour change, so it is well covered by the existing tests: any drift in a status code, result code or header would fail them.
Worth doing before #3365 rather than after.
What happens
src/mock_vws/_services_validators/exceptions.pyis 889 lines for 23 exception classes, andsrc/mock_vws/_query_validators/exceptions.pyis 769 lines for 21. That is 1658 lines and 44 classes, averaging 38 lines each, and almost all of it is the same three steps repeated:Grouping the classes by which headers they set:
So 20 of the 23 services exceptions differ from each other in exactly two values: the status code and the result code. The query exceptions vary more, but still fall into a handful of shapes.
Why it matters
This is where header changes have to be made, and it is why they are expensive. #3365 opens by noting "There are 39 occurrences across
src/" of thex-aws-regionvalue, and most of those are here. Whatever is decided in that issue is a 39-site edit against the current structure and a one-line edit against a factored one. The same applies toserver: envoy,x-envoy-upstream-service-timeandstrict-transport-security, which are equally hardcoded and equally likely to need revisiting.The repetition also hides the interesting information. What is worth reading about
UnknownTargetErroris "404,UnknownTarget". That fact is currently surrounded by 30 lines which are identical to the previous class.Suggested resolution
Move the response construction into the
ValidatorErrorbase class, so that the common case becomes a declaration:with the base building
transaction_id,response_textand the standard header set, and hooks for the minority of classes which need a different content type, an extra header such asWWW-Authenticate, or a non-JSON body.The two modules should keep their own base, since the services and query APIs genuinely have different standard header sets — the query API responses do not carry
x-aws-region, for instance. Sharing theDate,Content-Lengthand content-type logic between the two bases is worthwhile; forcing one header set on both is not.This is a pure refactor with no behaviour change, so it is well covered by the existing tests: any drift in a status code, result code or header would fail them.
Worth doing before #3365 rather than after.