Remove default dict parameters from api#236
Conversation
| kind (str): Specify kind="guest" to register as guest. | ||
| """ | ||
| if content is None: | ||
| content = {} |
There was a problem hiding this comment.
I'm confused, it causes havoc how? And as far as I can tell the outcome is exactly the same here.
There was a problem hiding this comment.
python will instantiate the argument once. After that any call to the method can mutate this single argument.
https://pythonconquerstheuniverse.wordpress.com/2012/02/15/mutable-default-arguments/
We've experienced this when running multiple clients in a single thread with gevent context switching. This has caused one client using access token of other client and other funny things.
There was a problem hiding this comment.
@pcppcp is this code doing gevent with multiple clients public anywhere by chance? Would be curious to see it.
There was a problem hiding this comment.
we use matrix as an experimental transport layer in Raiden and since in the tests there are sometimes many clients running in a single thread, a lot of context switching occurs and this had revealed the problem with the default dict.
There was a problem hiding this comment.
@non-Jedi we plan to upstream some of our changes in the near future, but if you're interested on some concepts of our usage, you may look at some of our usages here
|
Thanks! Been meaning to fix this well-known gotcha for a while now. Much appreciated. Could you please sign off per contributing.rst? |
pcppcp
force-pushed
the
remove_default_dict
branch
from
7f8ea2f to
1043cdc
Compare
|
@non-Jedi I've added this |
Signed-off-by: Jozef Henzl <popcorp86@gmail.com>
pcppcp
force-pushed
the
remove_default_dict
branch
from
1043cdc to
a135c05
Compare
|
@pcppcp yep. Thanks again! |
To enforce version including matrix-org/matrix-python-sdk#236 from @pcppcp
4 participants
Using dict or list as a default argument generally causes havoc. This PR removes them.