[GitHub] [incubator-superset] mistercrunch commented on a change in pull request #10844: Feat: Adding table comment and columns comment for SQLLab

[GitBox <gi...@apache.org>]

mistercrunch commented on a change in pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#discussion_r487582272



##########
File path: superset/db_engine_specs/base.py
##########
@@ -655,6 +655,26 @@ def get_view_names(
             views = [re.sub(f"^{schema}\\.", "", view) for view in views]
         return sorted(views)
 
+    @classmethod
+    def get_table_comment(
+        cls, inspector: Inspector, table_name: str, schema: Optional[str]
+    ) -> Optional[str]:
+        """
+        Get comment of table from a given schema and table
+
+        :param inspector: SqlAlchemy Inspector instance
+        :param table_name: Table name
+        :param schema: Schema name. If omitted, uses default schema for database
+        :return: comment of table
+        """
+        comment = None
+        try:
+            comment = inspector.get_table_comment(table_name, schema)
+            comment = comment.get("text") if isinstance(comment, dict) else None
+        except NotImplementedError:

Review comment:
       I think we need a wider catch here, and it could be good to alert on unexpected errors as in
   ```python
   except NotImplementedError:
       # It's expected that some dialects don't implement the comment fetching method
       pass
   except Exception as e:
       logging.error("Unexpected error while fetching table comment")
       logging.exception(e)
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org