Fix pylint unsupported assignment operation for session.retry[404] = 5

[jjm]

• Only define retry as {} once, instead of redefining None to {} (Fixes: #32).
• Bump version number to 4.0.1.

[jjm]

@Deconstrained Hi, would it be possible to get a review of this PR? This would help correct a couple of annoying linting errors we are seeing in our codebase using pdpyras.

[Deconstrained]

Hi! Sorry for the late reply as I have been ensconced in another project lately.

I am slightly hesitant here because of how the Python interpreter treats {} in class definitions and function signatures - the mutable defaults behavior. In a nutshell, all instances of the object will in that property have a pointer to a single object that is created in the definition of the class itself, unless that property is overridden with a new object.

Example:

$ python
Python 3.8.1 (default, Feb 11 2020, 11:10:51) 
[Clang 11.0.0 (clang-1100.0.33.17)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> class Foo:
...     foo = {}
... 
>>> a = Foo()
>>> a.foo['abc'] = 123
>>> b = Foo()
>>> b.foo
{'abc': 123}

Granted, while this does make it a bit easier to declare uniform custom retry behavior across instances, it also makes it easier to do this accidentally, and implementing this behavior wasn't my intention. My reasons for explicitly starting it out as None and then set it to an empty dict in __init__ are thus:

• Principle of least astonishment
• Avoiding dependence on a feature / behavior of Python that is not very obvious and may change in the future

However, to get around #32 , I think it might be possible to define the property using getter/setter methods and have it pass pylint tests, so I'm going to investigate that.

[jjm]

I had not thought of mutable defaults applying at a class level, however is this not a case of Class vs Instance Variables?

As it stands pdpyras is creating an class variable for retry set to None which then has a instance variable created set to {}? Which is used instead? Some checks using pdpyras 4.0:

>>> a = APISession(api_key="a")
>>> print(a.__class__.retry)
None
>>> print(a.retry)
{}
>>> vars(a)
{'api_call_counts': {}, 'api_time': {}, '_auth_type': 'token', 'parent': <super: <class 'PDSession'>, <APISession object>>, 'headers': {'User-Agent': 'python-requests/2.23.0', 'Accept-Encoding': 'gzip, deflate', 'Accept': 'application/vnd.pagerduty+json;version=2', 'Connection': 'keep-alive', 'Authorization': 'Token token=a'}, 'auth': None, 'proxies': {}, 'hooks': {'response': []}, 'params': {}, 'stream': False, 'verify': True, 'cert': None, 'max_redirects': 30, 'trust_env': True, 'cookies': <RequestsCookieJar[]>, 'adapters': OrderedDict([('https://', <requests.adapters.HTTPAdapter object at 0x1044dde10>), ('http://', <requests.adapters.HTTPAdapter object at 0x1044ddf10>)]), '_api_key': 'a', '_subdomain': None, 'log': <Logger pdpyras.APISession(*a) (WARNING)>, 'retry': {}, 'default_from': None}

Here is the same examples on this PR, with an what happens when retry has a new entry added and then replaced:

>>> from pdpyras import APISession
>>> a = APISession(api_key="a")
>>> print(a.retry)
{}
>>> print(a.__class__.retry)
{}
>>> vars(a)
{'api_call_counts': {}, 'api_time': {}, '_auth_type': 'token', 'parent': <super: <class 'PDSession'>, <APISession object>>, 'headers': {'User-Agent': 'python-requests/2.23.0', 'Accept-Encoding': 'gzip, deflate', 'Accept': 'application/vnd.pagerduty+json;version=2', 'Connection': 'keep-alive', 'Authorization': 'Token token=a'}, 'auth': None, 'proxies': {}, 'hooks': {'response': []}, 'params': {}, 'stream': False, 'verify': True, 'cert': None, 'max_redirects': 30, 'trust_env': True, 'cookies': <RequestsCookieJar[]>, 'adapters': OrderedDict([('https://', <requests.adapters.HTTPAdapter object at 0x10f0bb590>), ('http://', <requests.adapters.HTTPAdapter object at 0x10f0bb650>)]), '_api_key': 'a', '_subdomain': None, 'log': <Logger pdpyras.APISession(*a) (WARNING)>, 'default_from': None}

>>> a.retry[429] = 5
>>> print(a.__class__.retry)
{429: 5}

>>> a.retry = {404: 5}
>>> vars(a)
{'api_call_counts': {}, 'api_time': {}, '_auth_type': 'token', 'parent': <super: <class 'PDSession'>, <APISession object>>, 'headers': {'User-Agent': 'python-requests/2.23.0', 'Accept-Encoding': 'gzip, deflate', 'Accept': 'application/vnd.pagerduty+json;version=2', 'Connection': 'keep-alive', 'Authorization': 'Token token=a'}, 'auth': None, 'proxies': {}, 'hooks': {'response': []}, 'params': {}, 'stream': False, 'verify': True, 'cert': None, 'max_redirects': 30, 'trust_env': True, 'cookies': <RequestsCookieJar[]>, 'adapters': OrderedDict([('https://', <requests.adapters.HTTPAdapter object at 0x10f0bb590>), ('http://', <requests.adapters.HTTPAdapter object at 0x10f0bb650>)]), '_api_key': 'a', '_subdomain': None, 'log': <Logger pdpyras.APISession(*a) (WARNING)>, 'default_from': None, 'retry': {404: 5}}

I agree that retry should be not shared across different instances of the class, so should I'll do a another PR to instead remove the class variable and only have an instance variable instead? As the class variable is redundant and always overridden?

[Deconstrained]

Hi! I'm really sorry for not getting to this sooner, but I think it's fine as long as setting the initial instance variable value is added back in. To prevent further wait I'm going to get that ball rolling myself.

[jjm]

Thanks @Deconstrained for merging my PR and correcting the backward compatibility issue.