-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: use local_var_params to avoid collision with api parameters #521
Conversation
@@ -82,62 +82,63 @@ class {{classname}}(object): | |||
returns the request thread. | |||
""" | |||
|
|||
local_var_params = locals() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO it's enough to start with catching local variables. Rest of the code don't use local variables directly, only via this local_var_params
. Other created local variables (for instance collection_formats
) can't interfere with these locals()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For collection_formats
, let's say if there's a required parameter named collection_format
or CollectionFormat
, I believe it will cause an issue with the local variable collection_format
in the method, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it should work correctly. locals()
creates an array with references to passed parameters. If you assign a new value to variable you only change its reference, previous references still exist in local_var_params. It works until you remember to use arguments via local_var_params.
def test1(param):
# array with references to local variables
local_var = locals()
# override param
param = {"my-new-dict": 123}
# use this new value
print(param, id(param))
#>> ({'my-new-dict': 123}, 140441605067112)
# but still we can use refeneces from local_var
print(local_var['param'], id(local_var['param']))
#>> ('orig-param', 140441605013440)
test1("orig-param")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tomplus thanks for performing some tests to confirm that 👍
Added missing Milestone: |
PR checklist
./bin/
to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh
and./bin/security/{LANG}-petstore.sh
if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\
.master
,3.1.x
,4.0.x
. Default:master
.Description of the PR
Resolve #348
PTAL: @wing328 @taxpon (2017/07) @frol (2017/07) @mbohlool (2017/07) @cbornet (2017/09) @kenjones-cisco (2017/11)