Pylint part 1 #2314

glennmatthews · 2022-08-25T19:54:34Z

Progress towards: #2292

What's Changed

Add pylint and pylint-django as dev dependencies
Add invoke pylint task and nautobot-server pylint command provided by the example plugin. (The latter is needed because of our custom startup code, meaning that DJANGO_SETTINGS_MODULE=nautobot.core.settings pylint nautobot/ doesn't work on its own)
Add example plugin to PLUGINS in the development config (closes Enable Example Plugin by Default in Developer Environment #2198) so that the above will work
Fix and/or spot-disable a bunch of relatively low-hanging pylint warnings throughout our code base. It may be easier to review these commit-by-commit rather than looking at the entire diffset at once.
- cabb177 bad-super-call,dangerous-default-value,expression-not-assigned,no-self-argument,trailing-comma-tuple
- 70947aa unbalanced-tuple-unpacking, undefined-loop-variable, unused-variable, used-before-assignment
- 64298f4 unnecessary-comprehension, unnecessary-pass, use-dict-literal, use-list-literal, useless-object-inheritance, useless-super-delegation, using-constant-test
- 4a3a49f abstract-class-instantiated, assignment-from-no-return, assignment-from-none, duplicate-key, function-redefined, implicit-str-concat
- c98694d arguments-differ, deprecated-decorator, deprecated-method, deprecated-module, inconsistent-return-statements, invalid-envvar-default, invalid-overridden-method, invalid-str-returned
- d997a25 redefined-argument-from-local, redefined-builtin, redefined-outer-name, redundant-unittest-assert, reimported
Disable all remaining current active pylint warnings against our code base so that we have a passing baseline.
- I've got a local branch that re-enables and addresses a bunch more of these, but the diffs get increasingly large and hard to follow, and probably more controversial as well. :-) . I'll open up follow-on PR(s) with those additional changes as time and interest permits, hence why this is a "part 1".
Make a first stab at adding pylint to CI - as it takes about 10 minutes to run, I've set it as a parallel step to the unit-tests, rather than parallel to the other, much faster linter steps.

TODO

Explanation of Change(s)
n/a Attached Screenshots, Payload Example
n/a Unit, Integration Tests
Documentation Updates (when adding/changing features)
git pre-commit hook updates
Example Plugin Updates (when adding/changing features)
Outline Remaining Work, Constraints from Design

…not-assigned,no-self-argument,trailing-comma-tuple

…nused-variable, used-before-assignment

…ict-literal, use-list-literal, useless-object-inheritance, useless-super-delegation, using-constant-test

…turn, assignment-from-none, duplicate-key, function-redefined, implicit-str-concat Also update pylint command to include additional sources

…-method, deprecated-module, inconsistent-return-statements, invalid-envvar-default, invalid-overridden-method, invalid-str-returned

…uter-name, redundant-unittest-assert, reimported

glennmatthews · 2022-08-26T13:28:02Z

nautobot/core/apps/__init__.py

@@ -190,7 +190,7 @@ def register_homepage_panels(path, label, homepage_layout):
                            )
                        else:
                            raise TypeError(f"Third level objects need to be an instance of HomePageItem: {group_item}")
-                    panel_perms |= set(group_item.permissions)
+                        panel_perms |= set(group_item.permissions)


Bugfix here - we were only adding the last group_item.permissions (after the above for loop terminates) instead of adding the permissions of each group_item in the loop.

glennmatthews · 2022-08-26T13:29:18Z

nautobot/core/runner/importer.py

@@ -8,27 +8,18 @@



Changes to this file are removing Python 2 backwards compatibility code that's no longer needed. There's probably more that could be done here but this is sufficient to address the immediate pylint warnings.

nautobot/core/runner/settings.py

glennmatthews · 2022-08-26T13:32:13Z

nautobot/core/views/mixins.py

@@ -84,7 +84,7 @@ def get_permissions_for_model(self, model, actions):
        for action in actions:
            if action not in ("view", "add", "change", "delete"):
                raise ValueError(f"Unsupported action: {action}")
-        permissions.append("{}.{}_{}".format(model._meta.app_label, action, model._meta.model_name))
+            permissions.append("{}.{}_{}".format(model._meta.app_label, action, model._meta.model_name))


Another bug fix where we were doing something after the for loop terminates instead of per-iteration of the loop.

nautobot/dcim/models/sites.py

nautobot/extras/jobs.py

nautobot/extras/scripts.py

nautobot/extras/views.py

nautobot/tenancy/models.py

nautobot/dcim/fields.py

gsnider2195

Lots of good fixes!

glennmatthews added 12 commits August 24, 2022 11:59

Add nautobot-server pylint command, disable all initially failing rules

c52e296

Enable and address bad-super-call,dangerous-default-value,expression-…

cabb177

…not-assigned,no-self-argument,trailing-comma-tuple

Enable and fix unbalanced-tuple-unpacking, undefined-loop-variable, u…

70947aa

…nused-variable, used-before-assignment

Enable and address unnecessary-comprehension, unnecessary-pass, use-d…

64298f4

…ict-literal, use-list-literal, useless-object-inheritance, useless-super-delegation, using-constant-test

Enable and address abstract-class-instantiated, assignment-from-no-re…

4a3a49f

…turn, assignment-from-none, duplicate-key, function-redefined, implicit-str-concat Also update pylint command to include additional sources

Enable and address arguments-differ, deprecated-decorator, deprecated…

c98694d

…-method, deprecated-module, inconsistent-return-statements, invalid-envvar-default, invalid-overridden-method, invalid-str-returned

Linting

380c636

Address redefined-argument-from-local, redefined-builtin, redefined-o…

d997a25

…uter-name, redundant-unittest-assert, reimported

Invoke task and CI

b8fe8fe

Update CI workflow

6b9cb33

Fix a couple things that crept in during rebases

3d3d3cd

Merge branch 'develop' into gfm-pylint-part-1

b876966

glennmatthews commented Aug 26, 2022

View reviewed changes

Address some items from review

1175e1b

bryanculver self-requested a review August 26, 2022 15:20

gsnider2195 self-requested a review August 26, 2022 15:21

glennmatthews mentioned this pull request Aug 26, 2022

Replace imp with importlib #2318

Merged

Doc and pre-commit hook updates

9e67ed7

gsnider2195 reviewed Aug 29, 2022

View reviewed changes

nautobot/dcim/fields.py Outdated Show resolved Hide resolved

glennmatthews mentioned this pull request Aug 29, 2022

Fix code and logic around MACAddressCharField #2332

Merged

2 tasks

gsnider2195 approved these changes Aug 29, 2022

View reviewed changes

Replace imp with importlib (#2318)

e3d5b4f

bryanculver approved these changes Aug 29, 2022

View reviewed changes

Fix code and logic around MACAddressCharField (#2332)

66da092

glennmatthews merged commit ab80c43 into develop Aug 29, 2022

glennmatthews deleted the gfm-pylint-part-1 branch August 29, 2022 19:33

glennmatthews added a commit that referenced this pull request Aug 29, 2022

Add release-note for #2314

22469d1

glennmatthews mentioned this pull request Sep 2, 2022

Enable pylint for local linting and CI #2292

Closed

bryanculver mentioned this pull request Sep 9, 2022

Cleanup Codebase Lint #2397

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pylint part 1 #2314

Pylint part 1 #2314

glennmatthews commented Aug 25, 2022 •

edited

Loading

glennmatthews Aug 26, 2022

glennmatthews Aug 26, 2022

glennmatthews Aug 26, 2022

gsnider2195 left a comment

Pylint part 1 #2314

Pylint part 1 #2314

Conversation

glennmatthews commented Aug 25, 2022 • edited Loading

Progress towards: #2292

What's Changed

TODO

glennmatthews Aug 26, 2022

Choose a reason for hiding this comment

glennmatthews Aug 26, 2022

Choose a reason for hiding this comment

glennmatthews Aug 26, 2022

Choose a reason for hiding this comment

gsnider2195 left a comment

Choose a reason for hiding this comment

glennmatthews commented Aug 25, 2022 •

edited

Loading