chore(pylint): Reenable import-outside-toplevel check

[john-bodley]

SUMMARY

Re-enabling the Pylint import-outside-toplevel check.

Note the presence of the # pylint: disable=import-outside-toplevel is an indication of the cyclical dependency logic which exists within our code based (stemming from the fact that superset/__init__.py used to house a significant amount of logic. I realize there have been efforts to remedy this, but it's fairly apparent there's lots more work needed here.

TESTING INSTRUCTIONS

CI.

ADDITIONAL INFORMATION

• Has associated issue: [SIP-46] Proposal for strict Pylint enforcement #9953
• 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

[john-bodley]

I'm kind of surprised Mypy didn't pick up on this, i.e., the db_uri could be None though the get_or_create_db is expecting a str rather than Optional[str].

[codecov]

Codecov Report

Merging #16263 (179750d) into master (36abc51) will increase coverage by 0.00%.
The diff coverage is 78.26%.

@@           Coverage Diff           @@
##           master   #16263   +/-   ##
=======================================
  Coverage   76.49%   76.49%           
=======================================
  Files         997      997           
  Lines       53252    53246    -6     
  Branches     6777     6777           
=======================================
- Hits        40734    40731    -3     
+ Misses      12288    12285    -3     
  Partials      230      230           

Flag	Coverage Δ
mysql	81.60% <78.26%> (+<0.01%)	⬆️
postgres	81.62% <78.26%> (+<0.01%)	⬆️
python	81.71% <78.26%> (+<0.01%)	⬆️
sqlite	81.23% <78.26%> (+<0.01%)	⬆️

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

Impacted Files	Coverage Δ
superset/connectors/sqla/models.py	88.04% <ø> (ø)
superset/dashboards/commands/importers/v0.py	89.94% <ø> (ø)
superset/db_engine_specs/bigquery.py	87.75% <ø> (ø)
superset/db_engine_specs/elasticsearch.py	90.24% <0.00%> (ø)
superset/db_engine_specs/mysql.py	97.61% <ø> (ø)
superset/db_engine_specs/presto.py	83.47% <ø> (ø)
superset/db_engines/hive.py	0.00% <ø> (ø)
superset/jinja_context.py	89.36% <ø> (ø)
superset/sql_validators/presto_db.py	82.89% <ø> (ø)
superset/tasks/async_queries.py	96.77% <ø> (ø)
... and 13 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 36abc51...179750d. Read the comment docs.

[villebro]

LGTM - While local imports are needed in Superset (e.g. when needing functionality from an optional dep), I agree that the linting rule should always be overridden when needed to ensure toplevel importing was considered.

[villebro]

I think cases like these are motivation enough for enabling this check - I assume this was unintentional, and was probably a misclick in an IDE (this context menu from PyCharm when clicking Option+Return):

[john-bodley]

@villebro agreed, though as pointed out in the description this check highlights the cyclical dependencies which unnecessarily exist and we should strive to eradicate.