Skip to content

Commit

Permalink
fix: several disabled pylint rules in models/helpers.py (#10909)
Browse files Browse the repository at this point in the history
* Removed conflicting lint and isort check in model helpers seems it's not appearing anymore

* Removed disabled linting for accessing private method. `parent_foreign_key_mappings` becomes public because it is accessed by other instance than `self`.

* Updated model's helper - removed unecessary exception and replaced with check while accessing global context to reset ownerships.

* Updated model's helper - renamed unused attribute to private in user link method.

* Updated model's helper - added specific exception for adding extra json column. Removed disabled pylint rule.

* Applied changes after review to `models/helpers.py`:
- removed unecesary function's param rename
- added extra JSON content in exception

* Removed self.extra_json content from exception message.
  • Loading branch information
kkucharc authored Sep 18, 2020
1 parent e375ed4 commit 2003442
Showing 1 changed file with 20 additions and 16 deletions.
36 changes: 20 additions & 16 deletions superset/models/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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]:
Expand All @@ -321,7 +322,7 @@ 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
def _user_link(user: User) -> Union[Markup, str]:
if not user:
return ""
url = "/superset/profile/{}/".format(user.username)
Expand Down Expand Up @@ -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
)
return {}

def set_extra_json(self, extras: Dict[str, Any]) -> None:
Expand Down

0 comments on commit 2003442

Please sign in to comment.