Fix usage of a single object instances for multiple concurrent requests #70
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dfangl
commented
Aug 31, 2023
moto/core/responses.py
Outdated
Comment on lines
237
to
253
| @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 | ||
| """ | ||
|
|
||
| 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.
This is the main logic used for this change.
dfangl
commented
Aug 31, 2023
| :return: A wrapper executing the given method on a new instance of this class | ||
| """ | ||
|
|
||
| @functools.wraps(to_call) # type: ignore |
There was a problem hiding this comment.
This is currently needed to pass the test test_core.test_server.test_domain_dispatched_with_service (with modifications). It not really wraps the method completely, so this should be discussed.
|
Merged upstream in getmoto#6746 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.