-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
fix: several disabled pylint rules in models/helpers.py #10909
Changes from 5 commits
aa608e9
858a275
aea3303
4632bb3
422c026
e22195b
17e3a54
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,10 +19,9 @@ | |
import logging | ||
import re | ||
from datetime import datetime, timedelta | ||
from json.decoder import JSONDecodeError | ||
from typing import Any, Dict, List, Optional, Set, Union | ||
|
||
# isort and pylint disagree, isort should win | ||
# pylint: disable=ungrouped-imports | ||
import humanize | ||
import pandas as pd | ||
import pytz | ||
|
@@ -45,9 +44,7 @@ | |
def json_to_dict(json_str: str) -> Dict[Any, Any]: | ||
if json_str: | ||
val = re.sub(",[ \t\r\n]+}", "}", json_str) | ||
val = re.sub( | ||
",[ \t\r\n]+\]", "]", val # pylint: disable=anomalous-backslash-in-string | ||
) | ||
val = re.sub(",[ \t\r\n]+\\]", "]", val) | ||
return json.loads(val) | ||
|
||
return {} | ||
|
@@ -89,6 +86,14 @@ def _unique_constrains(cls) -> List[Set[str]]: | |
) | ||
return unique | ||
|
||
@classmethod | ||
def parent_foreign_key_mappings(cls) -> Dict[str, str]: | ||
"""Get a mapping of foreign name to the local name of foreign keys""" | ||
parent_rel = cls.__mapper__.relationships.get(cls.export_parent) | ||
if parent_rel: | ||
return {l.name: r.name for (l, r) in parent_rel.local_remote_pairs} | ||
return {} | ||
|
||
@classmethod | ||
def export_schema( | ||
cls, recursive: bool = True, include_parent_ref: bool = False | ||
|
@@ -135,7 +140,7 @@ def import_from_dict( | |
"""Import obj from a dictionary""" | ||
if sync is None: | ||
sync = [] | ||
parent_refs = cls._parent_foreign_key_mappings() | ||
parent_refs = cls.parent_foreign_key_mappings() | ||
export_fields = set(cls.export_fields) | set(parent_refs.keys()) | ||
new_children = {c: dict_rep[c] for c in cls.export_children if c in dict_rep} | ||
unique_constrains = cls._unique_constrains() | ||
|
@@ -217,9 +222,7 @@ def import_from_dict( | |
# If children should get synced, delete the ones that did not | ||
# get updated. | ||
if child in sync and not is_new_obj: | ||
back_refs = ( | ||
child_class._parent_foreign_key_mappings() # pylint: disable=protected-access | ||
) | ||
back_refs = child_class.parent_foreign_key_mappings() | ||
delete_filters = [ | ||
getattr(child_class, k) == getattr(obj, back_refs.get(k)) | ||
for k in back_refs.keys() | ||
|
@@ -306,11 +309,9 @@ def reset_ownership(self) -> None: | |
self.created_by = None | ||
self.changed_by = None | ||
# flask global context might not exist (in cli or tests for example) | ||
try: | ||
if g.user: | ||
self.owners = [g.user] | ||
except Exception: # pylint: disable=broad-except | ||
self.owners = [] | ||
self.owners = [] | ||
if g and hasattr(g, "user"): | ||
self.owners = [g.user] | ||
|
||
@property | ||
def params_dict(self) -> Dict[Any, Any]: | ||
|
@@ -321,11 +322,11 @@ def template_params_dict(self) -> Dict[Any, Any]: | |
return json_to_dict(self.template_params) # type: ignore | ||
|
||
|
||
def _user_link(user: User) -> Union[Markup, str]: # pylint: disable=no-self-use | ||
if not user: | ||
def _user_link(_user: User) -> Union[Markup, str]: | ||
if not _user: | ||
return "" | ||
url = "/superset/profile/{}/".format(user.username) | ||
return Markup('<a href="{}">{}</a>'.format(url, escape(user) or "")) | ||
url = "/superset/profile/{}/".format(_user.username) | ||
return Markup('<a href="{}">{}</a>'.format(url, escape(_user) or "")) | ||
|
||
|
||
class AuditMixinNullable(AuditMixin): | ||
|
@@ -424,7 +425,10 @@ class ExtraJSONMixin: | |
def extra(self) -> Dict[str, Any]: | ||
try: | ||
return json.loads(self.extra_json) | ||
except Exception: # pylint: disable=broad-except | ||
except (TypeError, JSONDecodeError) as exc: | ||
logger.error( | ||
"Unable to load an extra json: %r. Leaving empty.", exc, exc_info=True | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea! I will add it to an exception logger. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe I did not explain myself right, sorry. I want to make sure that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dpgaspar do you mind to take a look again? I have one doubt: do you think is it possible there could be raised exception while accessing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Och, I see, then I just did the opposite thing! :) Reverting this change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to be sure I check that and any of them won't show what There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice then, thks |
||
) | ||
return {} | ||
|
||
def set_extra_json(self, extras: Dict[str, Any]) -> None: | ||
|
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't we just remove the
# pylint: disable=no-self-use
on this one?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.
You are absolutely right! Somehow I had to mistake that for
unused-argument
. Thanks! I'm changing it right-away.