chore(ci): bump pylint to 2.10.2 #16463

villebro · 2021-08-26T10:18:16Z

SUMMARY

While working on an unrelated PR, I started getting weird linting errors from pylint that flip-flopped between

pylint run-test: commands[0] | pylint superset
************* Module superset.models.slice
superset/models/slice.py:56:0: R0902: Too many instance attributes (10/8) (too-many-instance-attributes)
************* Module superset.models.dashboard
superset/models/dashboard.py:132:0: R0902: Too many instance attributes (9/8) (too-many-instance-attributes)

and later when those were disabled

pylint run-test: commands[0] | pylint superset
************* Module superset.models.slice
superset/models/slice.py:56:0: I0021: Useless suppression of 'too-many-instance-attributes' (useless-suppression)
************* Module superset.models.dashboard
superset/models/dashboard.py:132:0: I0021: Useless suppression of 'too-many-instance-attributes' (useless-suppression)

I figured this might have been fixed in the most recent 2.10.2 release of pylint, but sadly that wasn't the case (same flip-flop). However, since I already did the necessary corrections for 2.10, I figure we might just as well bump anyway.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

villebro · 2021-08-26T10:20:09Z

.pylintrc

@@ -86,6 +86,8 @@ disable=
    missing-docstring,
    too-many-lines,
    duplicate-code,
+    unspecified-encoding,


I don't necessarily agree with this rule. While we could add encoding="utf-8" to all text file opens, this would likely break compatibility with Windows which defaults to cp-1252 (not officially supported, but I know some parties in the community are running Superset on Windows). Also, adding encoding=locale.getpreferredencoding() feels pointless. So I recommend we disable this rule for now.

villebro · 2021-08-26T10:28:14Z

superset/annotation_layers/annotations/commands/create.py

-        exceptions: List[ValidationError] = list()
+        exceptions: List[ValidationError] = []


Ok, this made me chuckle. I don't remember which linter and when, but didn't some linter up until fairly recently complain about using [] and recommend using list() instead? 😆 See use-list-literal (R1734) and use-dict-literal (R1735) in http://pylint.pycqa.org/en/latest/technical_reference/features.html
(FYI: I much prefer [] and {} over list() and dict() respectively, so I'm really happy about this change)

codecov · 2021-08-26T10:40:36Z

Codecov Report

Merging #16463 (bfcea63) into master (18be181) will decrease coverage by 0.24%.
The diff coverage is 77.77%.

❗ Current head bfcea63 differs from pull request most recent head 0563634. Consider uploading reports for the commit 0563634 to get more accurate results

@@            Coverage Diff             @@
##           master   #16463      +/-   ##
==========================================
- Coverage   76.63%   76.38%   -0.25%     
==========================================
  Files        1002     1002              
  Lines       53637    53639       +2     
  Branches     6853     6854       +1     
==========================================
- Hits        41104    40972     -132     
- Misses      12294    12428     +134     
  Partials      239      239

Flag	Coverage Δ
hive	`?`
mysql	`?`
postgres	`81.55% <86.20%> (-0.04%)`	⬇️
presto	`?`
python	`81.59% <86.20%> (-0.52%)`	⬇️
sqlite	`81.19% <82.14%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...perset-frontend/src/explore/components/Control.tsx	`76.47% <ø> (ø)`
...-frontend/src/explore/components/ControlHeader.jsx	`86.20% <ø> (ø)`
superset-frontend/src/views/CRUD/types.ts	`100.00% <ø> (ø)`
superset/views/base_schemas.py	`0.00% <0.00%> (ø)`
...ontrols/DndColumnSelectControl/DndMetricSelect.tsx	`40.88% <20.00%> (-0.53%)`	⬇️
superset/annotation_layers/annotations/api.py	`84.96% <50.00%> (ø)`
superset/utils/dict_import_export.py	`75.00% <50.00%> (ø)`
superset/tasks/async_queries.py	`96.77% <66.66%> (ø)`
...d/src/explore/components/DatasourcePanel/index.tsx	`73.95% <69.23%> (ø)`
...ontrols/DndColumnSelectControl/DndColumnSelect.tsx	`47.56% <100.00%> (ø)`
... and 35 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 18be181...0563634. Read the comment docs.

john-bodley · 2021-08-26T17:24:56Z

superset/annotation_layers/annotations/api.py

@@ -381,7 +381,7 @@ def put(  # pylint: disable=arguments-differ
        try:
            new_model = UpdateAnnotationCommand(g.user, annotation_id, item).run()
            return self.response(200, id=new_model.id, result=item)
-        except (AnnotationNotFoundError, AnnotationLayerNotFoundError) as ex:
+        except (AnnotationNotFoundError, AnnotationLayerNotFoundError):


I'm always happy to see Pylint is continuing to evolve it's logic.

I was actually really surprised this wasn't caught by older versions

john-bodley · 2021-08-26T17:26:19Z

superset/tasks/async_queries.py

@@ -68,7 +68,7 @@ def load_chart_data_into_cache(
        raise exc
    except Exception as exc:
        # TODO: QueryContext should support SIP-40 style errors
-        error = exc.message if hasattr(exc, "message") else str(exc)  # type: ignore # pylint: disable=no-member
+        error = exc.message if hasattr(exc, "message") else str(exc)  # type: ignore


@villebro as a side note I might do a pass to remain exc to ex for consistency throughout the code.

* upstream/master: (25 commits) chore(ci): bump pylint to 2.10.2 (apache#16463) fix: prevent page crash when chart can't render (apache#16464) chore: fixed slack invite link (apache#16466) fix(native-filters): handle null values in value filter (apache#16460) feat: add function list to auto-complete to Clickhouse datasource (apache#16234) refactor(explore): improve typing for Dnd controls (apache#16362) fix(explore): update overwrite button on perm change (apache#16437) feat: Draggable and Resizable Modal (apache#16394) refactor: sql_json view endpoint (apache#16441) fix(dashboard): undo and redo buttons weird alignment (apache#16417) fix: setupPlugin in chart list page (apache#16413) fix: Disable Slack notification method if no api token (apache#16367) feat: add Shillelagh DB engine spec (apache#16416) fix: copy to Clipboard order (apache#16299) docs: make FEATURE_FLAGS.md reference a link (apache#16415) chore(viz): bump superset-ui to 0.17.87 (apache#16420) feat: add activate command (apache#16404) Revert "fix(explore): let admin overwrite slice (apache#16290)" (apache#16408) fix(explore): retain chart ownership on query context update (apache#16419) chore: Removes the TODOs and uses the default page size (apache#16422) ...

superset-github-bot bot added the preset-io label Aug 26, 2021

pull-request-size bot added the size/M label Aug 26, 2021

villebro commented Aug 26, 2021

View reviewed changes

chore(ci): bump pylint to 2.10.2

0563634

villebro force-pushed the villebro/pylint-2.10.2 branch from 2e748b3 to 0563634 Compare August 26, 2021 10:20

villebro requested a review from john-bodley August 26, 2021 10:22

villebro commented Aug 26, 2021

View reviewed changes

amitmiran137 approved these changes Aug 26, 2021

View reviewed changes

john-bodley approved these changes Aug 26, 2021

View reviewed changes

john-bodley reviewed Aug 26, 2021

View reviewed changes

villebro merged commit ac1d779 into apache:master Aug 26, 2021

villebro mentioned this pull request Sep 6, 2021

2.10.2 Hangs forever, downgrade to 2.9.6 is working pylint-dev/pylint#4972

Closed

opus-42 pushed a commit to opus-42/incubator-superset that referenced this pull request Nov 14, 2021

chore(ci): bump pylint to 2.10.2 (apache#16463)

cf3eee9

QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 28, 2021

chore(ci): bump pylint to 2.10.2 (apache#16463)

fdc86c5

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.4.0 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(ci): bump pylint to 2.10.2 #16463

chore(ci): bump pylint to 2.10.2 #16463

villebro commented Aug 26, 2021

villebro Aug 26, 2021 •

edited

villebro Aug 26, 2021 •

edited

codecov bot commented Aug 26, 2021

john-bodley Aug 26, 2021

villebro Aug 26, 2021

john-bodley Aug 26, 2021

		exceptions: List[ValidationError] = list()
		exceptions: List[ValidationError] = []

chore(ci): bump pylint to 2.10.2 #16463

chore(ci): bump pylint to 2.10.2 #16463

Conversation

villebro commented Aug 26, 2021

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

villebro Aug 26, 2021 • edited

Choose a reason for hiding this comment

villebro Aug 26, 2021 • edited

Choose a reason for hiding this comment

codecov bot commented Aug 26, 2021

Codecov Report

john-bodley Aug 26, 2021

Choose a reason for hiding this comment

villebro Aug 26, 2021

Choose a reason for hiding this comment

john-bodley Aug 26, 2021

Choose a reason for hiding this comment

villebro Aug 26, 2021 •

edited

villebro Aug 26, 2021 •

edited