-
Notifications
You must be signed in to change notification settings - Fork 14.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Re-enable pylint for superset/utils folder #8766
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8766 +/- ##
==========================================
- Coverage 65.83% 65.83% -0.01%
==========================================
Files 482 482
Lines 24155 24153 -2
Branches 2770 2770
==========================================
- Hits 15903 15901 -2
Misses 8074 8074
Partials 178 178
Continue to review full report at Codecov.
|
@@ -30,7 +29,7 @@ def import_datasource( | |||
superset instances. Audit metadata isn't copies over. | |||
""" | |||
make_transient(i_datasource) | |||
logging.info("Started import of the datasource: {}".format(i_datasource.to_json())) | |||
logging.info("Started import of the datasource: %s", i_datasource.to_json()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use f strings instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not according to pylint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fstrings and .format
are not recommended for logging statements because they make you pay the cost of formatting the string regardless of logging level. Not that it's expensive, but it's best practice to use the format above for that reason
CATEGORY
Choose one
SUMMARY
Re-enables pylint for a few files in the superset/utils folder.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
ADDITIONAL INFORMATION
REVIEWERS
@john-bodley @dpgaspar @mistercrunch