-
Notifications
You must be signed in to change notification settings - Fork 10
Do not use mutable default arguments #383
Do not use mutable default arguments #383
Conversation
@microsoft-github-policy-service agree |
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.
Hi @marlamb,
Thanks for using kiota and for starting this work.
First off, here is the link to the article https://docs.python-guide.org/writing/gotchas/?highlight=mutable%20default#mutable-default-arguments
This is probably an issue we have across all the kiota Python repositories. And maybe in multiple places here.
We'd be more than happy to receive pull requests for other defects.
For this one, to ensure a swift release, can you also bump the patch version in the project file and add an entry in the changelog please?
97eaf69
to
82462ca
Compare
Yes, it seems to be a common problem. I just opened two other pull requests (outside kiota, but within the microsoft org)
Sorry for mistyping the link, I fixed it in the PR description. I added the changes as you request, please have a look if it meets your expectations. |
Using mutable default arguments is a common Python problem, see e.g. https://docs.python-guide.org/writing/gotchas/#mutable-default-arguments In this specific case the default argument even tries to setup some infrastructure settings at import time, which can potentially fail.
82462ca
to
ffdcdb9
Compare
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.
Thanks for making the changes!
Using mutable default arguments is a common Python problem, see e.g. https://docs.python-guide.org/writing/gotchas/#mutable-default-arguments
In this specific case the default argument even tries to setup some infrastructure settings at import time, which can potentially fail.