FIX: Don't default to list in method args #2518
Conversation
Fixes @list_route and @detail_route so that they don't initialize their `methods` parameter as a list. In some cases the list gets cleared, and the result is that default parameter is now empty, and may get reused unexpectedly.
|
Good catch though I'd probably rather have them as a tuple to avoid mutability. |
| """ | ||
| Used to mark a method on a ViewSet that should be routed for detail requests. | ||
| """ | ||
| if methods is None: | ||
| methods = ['get'] |
There was a problem hiding this comment.
Add a newline here, also let's use the methods = ['get'] if (methods is None) else methods style.
| """ | ||
| Used to mark a method on a ViewSet that should be routed for detail requests. | ||
| """ | ||
| methods = ['get'] if methods is None else methods |
There was a problem hiding this comment.
Minor one, this, but I tend to prefer using brackets around the if clause for visual clarity, so I normally do...
['get'] if (methods is None) else methods
|
I'll take this either with or without tuples. Happy for that to be discussed separately as it's a slightly different issue. |
This fix is clearly correct, but I'd be interested to know in what context the list gets mutated? |
|
Regarding tuples, I chose not to use them because I didn't want to mess with any code which is trying to change the list (even if it shouldn't be), I didn't want to go down that path right now. I'm using a pretty standard setup and I just happened to notice that if I used: it just didn't show up. Digging showed me that the methods param was empty. |
|
Okay, so just the brackets around the if clause for visual clarity and then I'm happy for this to go in. |
FIX: Don't default to list in method args
|
Loverly. |
|
Thanks for tracking this down, this is a very tricky Python side effect. |
|
👍 it has caught me a few times |
It did work for me if used as |
Fixes
@list_routeand@detail_routeso that they don't initialize theirmethodsparameter as a list. In some cases the list gets cleared, and the result is that default parameter is now empty, and may get reused unexpectedly.