From 2003442b3233b6d9a6969660b25960f2da472fb4 Mon Sep 17 00:00:00 2001
From: Kasia Kucharczyk <2536609+kkucharc@users.noreply.github.com>
Date: Fri, 18 Sep 2020 17:32:02 +0200
Subject: [PATCH] fix: several disabled pylint rules in models/helpers.py
 (#10909)

* 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.
---
 superset/models/helpers.py | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index 105a727761..fd55bba5ce 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -19,10 +19,9 @@ import json
 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 @@ logger = logging.getLogger(__name__)
 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 @@ class ImportMixin:
         )
         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 @@ class ImportMixin:
         """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 @@ class ImportMixin:
                 # 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 @@ class ImportMixin:
         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,7 +322,7 @@ class ImportMixin:
         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)
@@ -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: