Fix usage of a single object instances for multiple concurrent requests #6746
Conversation
| @classmethod | ||
| def method_dispatch( # type: ignore[misc] | ||
| cls, to_call: Callable[[ResponseShape, Any, str, Any], TYPE_RESPONSE] | ||
| ) -> Callable[[Any, str, Any], TYPE_RESPONSE]: | ||
| """ | ||
| Takes a given unbound function (part of a Response class) and executes it for a new instance of this | ||
| response class. | ||
| Can be used wherever we want to specify different methods for dispatching in urls.py | ||
| :param to_call: Unbound method residing in this Response class | ||
| :return: A wrapper executing the given method on a new instance of this class | ||
| """ | ||
|
|
||
| @functools.wraps(to_call) # type: ignore | ||
| def _inner(request: Any, full_url: str, headers: Any) -> TYPE_RESPONSE: | ||
| return getattr(cls(), to_call.__name__)(request, full_url, headers) | ||
|
|
||
| return _inner |
There was a problem hiding this comment.
Main logic. We might want to rethink the functools.wraps here, but it is necessary for tests like test_core.test_server.test_domain_dispatched_with_service, although it still needed a change, due to the different function name.
| "{0}/workspaces/(?P<workspace_id>[^/]+)/rulegroupsnamespaces/(?P<name>[^/]+)$": PrometheusServiceResponse.dispatch, | ||
| "{0}/tags/(?P<resource_arn>[^/]+)$": PrometheusServiceResponse.dispatch, | ||
| "{0}/tags/(?P<arn_prefix>[^/]+)/(?P<workspace_id>[^/]+)$": PrometheusServiceResponse.method_dispatch( | ||
| PrometheusServiceResponse.tags # type: ignore |
There was a problem hiding this comment.
This method returns a string, which is why the type ignore has to be here. There are some which do not match the type we now check, but most are fine.
| @@ -43,4 +43,4 @@ def test_domain_dispatched_with_service(): | |||
| dispatcher = DomainDispatcherApplication(create_backend_app, service="s3") | |||
| backend_app = dispatcher.get_application({"HTTP_HOST": "s3.us-east1.amazonaws.com"}) | |||
| keys = set(backend_app.view_functions.keys()) | |||
| assert "S3Response.key_response" in keys | |||
| assert "moto.s3.responses.key_response" in keys | |||
There was a problem hiding this comment.
This needed to be change since it is now a function, and not a method anymore. This test also necessitates the functools.wrap in the BaseResponse method_dispatch method.
| @@ -62,6 +62,7 @@ def _vault_response_delete(self, vault_name: str, headers: Any) -> TYPE_RESPONSE | |||
| def vault_archive_response( | |||
| self, request: Any, full_url: str, headers: Any | |||
| ) -> TYPE_RESPONSE: | |||
| self.setup_class(request, full_url, headers, use_raw_body=True) | |||
There was a problem hiding this comment.
This was missing here, I assume it never occurred, since there are other calls using glacier and setting up the class before. Now it is a new one, which leads to this new line.
Codecov Report
@@ Coverage Diff @@
## master #6746 +/- ##
==========================================
- Coverage 96.20% 96.20% -0.01%
==========================================
Files 808 808
Lines 79576 79565 -11
==========================================
- Hits 76557 76546 -11
Misses 3019 3019
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
This is now part of moto >= 4.2.1.dev29 |
Fixes #6745
Motivation
Currently, for the following services:
we use a single instance of the respective
...Responseclass to handle all the requests.In LocalStack, this leads to issues with parallel requests influencing each others state, most notably of region and account.
There are a couple of different ways to fix this, we chose one that was minimally intrusive to the existing logic, but ensured a separate instance of the Response class for every request.
Unaffected services use the
dispatchmethod. There are some services which use the classmethoddispatchon an instance, but since that does not make a difference (even though it can be confusing), we did not change it with this PR.Changes
method_dispatchclass method in the BaseResponse class, which will take an unbound function as input, and execute the corresponding method on a new instance on the class. This ensures proper typing and having the type checker catch potential errors, while being able to keep the current pattern of dispatching.