{"action":"created","issue":{"url":"https://api.github.com/repos/firehol/netdata/issues/4025","repository_url":"https://api.github.com/repos/firehol/netdata","labels_url":"https://api.github.com/repos/firehol/netdata/issues/4025/labels{/name}","comments_url":"https://api.github.com/repos/firehol/netdata/issues/4025/comments","events_url":"https://api.github.com/repos/firehol/netdata/issues/4025/events","html_url":"https://github.com/firehol/netdata/pull/4025","id":346740847,"node_id":"MDExOlB1bGxSZXF1ZXN0MjA1NTMxOTU3","number":4025,"title":"Small cleanups to python.d plugin module code.","user":{"login":"Ferroin","id":905151,"node_id":"MDQ6VXNlcjkwNTE1MQ==","avatar_url":"https://avatars2.githubusercontent.com/u/905151?v=4","gravatar_id":"","url":"https://api.github.com/users/Ferroin","html_url":"https://github.com/Ferroin","followers_url":"https://api.github.com/users/Ferroin/followers","following_url":"https://api.github.com/users/Ferroin/following{/other_user}","gists_url":"https://api.github.com/users/Ferroin/gists{/gist_id}","starred_url":"https://api.github.com/users/Ferroin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Ferroin/subscriptions","organizations_url":"https://api.github.com/users/Ferroin/orgs","repos_url":"https://api.github.com/users/Ferroin/repos","events_url":"https://api.github.com/users/Ferroin/events{/privacy}","received_events_url":"https://api.github.com/users/Ferroin/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":27,"created_at":"2018-08-01T19:42:44Z","updated_at":"2018-08-06T12:07:35Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/firehol/netdata/pulls/4025","html_url":"https://github.com/firehol/netdata/pull/4025","diff_url":"https://github.com/firehol/netdata/pull/4025.diff","patch_url":"https://github.com/firehol/netdata/pull/4025.patch"},"body":"Both of these are reasonably simple cleanups.\r\n\r\nThe first removes unnecessary explicit inheritance from the builtin `object` type.  As explained in the commit, this inheritance is implicit in all Python classes, and thus making it explicit is generally discouraged.  Our own code only did it in 3 files.  The PyYAML and urllib3 modules bundled with Netdata do this all over the place, but I opted not to change those so that they are kept as close to the upstream versions as possible.\r\n\r\nThe second makes sure all the module's Service classes have initial assignments of each of their instance attributes in their `__init__` method.  This has the simple stylistic benefit of making sure it's known what instance attributes this class defines.  It also has a practical benefit that it avoids the possibility of needing to reallocate an instance's attribute dictionary (stored in the implicit `__dict__` attribute) after the initialization of the instance, which removes a possible source of latency (reallocating dictionaries in Python is _slow_).\r\n\r\nNeither of these introduces any functional changes other than the aforementioned removal of a potential latency source."},"comment":{"url":"https://api.github.com/repos/firehol/netdata/issues/comments/410686094","html_url":"https://github.com/firehol/netdata/pull/4025#issuecomment-410686094","issue_url":"https://api.github.com/repos/firehol/netdata/issues/4025","id":410686094,"node_id":"MDEyOklzc3VlQ29tbWVudDQxMDY4NjA5NA==","user":{"login":"Ferroin","id":905151,"node_id":"MDQ6VXNlcjkwNTE1MQ==","avatar_url":"https://avatars2.githubusercontent.com/u/905151?v=4","gravatar_id":"","url":"https://api.github.com/users/Ferroin","html_url":"https://github.com/Ferroin","followers_url":"https://api.github.com/users/Ferroin/followers","following_url":"https://api.github.com/users/Ferroin/following{/other_user}","gists_url":"https://api.github.com/users/Ferroin/gists{/gist_id}","starred_url":"https://api.github.com/users/Ferroin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Ferroin/subscriptions","organizations_url":"https://api.github.com/users/Ferroin/orgs","repos_url":"https://api.github.com/users/Ferroin/repos","events_url":"https://api.github.com/users/Ferroin/events{/privacy}","received_events_url":"https://api.github.com/users/Ferroin/received_events","type":"User","site_admin":false},"created_at":"2018-08-06T12:07:35Z","updated_at":"2018-08-06T12:07:35Z","author_association":"CONTRIBUTOR","body":"@paulfantom Entirely agreed that this should have been separate PR's.  Given that most of the discussion here is about the whole `object` inheritance thing, I'll just remove the second commit and update the PR description.\r\n\r\nAs far as checking that things work, not thoroughly enough.  As @l2isbad pointed out, if settable/deletable properties are being used, we need new-style classes.\r\n\r\nAt this point though, I'd actually argue for _adding_ explicit inheritance from `object` for all the classes we have in modules that aren't inheriting from anything currently.  Without that, we get old-style classes for them in Python 2 (and I had forgotten about this distinction when I was making this PR, because TBH I rarely deal with Python 2 outside of Netdata anymore), which means that not explicitly inheriting from `object` gives us different behavior on Python 2 from Python 3 (and I am strongly of the opinion that slightly less desirable style is tolerable in favor of avoiding behavioral differences across multiple versions of the underlying platform).\r\n\r\nAlso, regarding language proliferation in Netdata, you've got my vote for deprecating Node.js and converting the existing modules to either Python or Go (if it's going to Python, I can _probably_ help with it some, I'm not an expert on JavaScript, but I know enough that I can generally understand what's going on provided nothing really complicated is being done).  I agree that we should try to keep things reasonable, and I also agree with @l2isbad's assessment that keeping Python support is a good thing long-term.  We could probably stand to also get rid of all the currently deprecated `charts.d` modules, though there are a couple that appear to still be unique to that (I can look at getting those converted to Python modules in the near future so we can just deprecate all of them, and in theory the whole framework).\r\n\r\n@l2isbad No, there are plenty of people, especially those new to Python, who _don't_  instinctively know about stuff like this.  The whole reason this came up is because these few isolated cases of inheriting from `object` _look out of place_ to me (and I had forgotten about old-style classes, because I pretty much never use Python 2 anywhere but here).  They still do, even with the knowledge that properties are being used and don't work otherwise.\r\n\r\nThe metaclass example was provided as an example of the same type of stylistic argument (the argument over always using `__new__` versus only using it when it's needed is functionally the same as the discussion we're having about whether to always explicitly inherit from `object` or only do so when needed).  My apologies if that wasn't clear."}}
{"actor":{"display_login":"Ferroin"}}