You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/09/11 16:13:36 UTC
[GitHub] [incubator-superset] zhaoyongjie opened a new pull request #10844: Feat: Adding table comment and columns comment for SQLLab
zhaoyongjie opened a new pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844
### SUMMARY
adding table comment and columns comment for SQLLab
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
----------------------------------------------------------------
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
[GitHub] [incubator-superset] ktmud commented on pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#issuecomment-691343783
----------------------------------------------------------------
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
[GitHub] [incubator-superset] ktmud commented on pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#issuecomment-691343783
Do you mind fixing the CI (simply run `black superset/`) and sharing a little bit how may this be used in the UI or API?
----------------------------------------------------------------
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
[GitHub] [incubator-superset] zhaoyongjie edited a comment on pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
zhaoyongjie edited a comment on pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#issuecomment-691368582
@ktmud
### for columns comment:
I plan to add a comment label closed to each column, similar to the current index icon, and then when there is a comment in this column, the "Comment ICON" will be displayed and the comment will be displayed in the popover/modal
### for table comment:
Similar to the column display. Add a table comment icon beside the table name text
----------------------------------------------------------------
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
[GitHub] [incubator-superset] zhaoyongjie commented on a change in pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#discussion_r487622467
##########
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:
thanks reviewed and comment.
----------------------------------------------------------------
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
[GitHub] [incubator-superset] mistercrunch commented on a change in pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by 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
[GitHub] [incubator-superset] zhaoyongjie edited a comment on pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
zhaoyongjie edited a comment on pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#issuecomment-691368582
----------------------------------------------------------------
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
[GitHub] [incubator-superset] zhaoyongjie commented on pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#issuecomment-691368582
----------------------------------------------------------------
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
[GitHub] [incubator-superset] mistercrunch merged pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
mistercrunch merged pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844
----------------------------------------------------------------
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
[GitHub] [incubator-superset] ktmud edited a comment on pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#issuecomment-691343783
Do you mind fixing the CI (simply run `black superset/`) and sharing a little bit how may this be used in the UI?
----------------------------------------------------------------
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
[GitHub] [incubator-superset] ktmud edited a comment on pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#issuecomment-691343783
----------------------------------------------------------------
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
[GitHub] [incubator-superset] mistercrunch commented on pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#issuecomment-692451462
----------------------------------------------------------------
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
[GitHub] [incubator-superset] zhaoyongjie edited a comment on pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
zhaoyongjie edited a comment on pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#issuecomment-691368582
----------------------------------------------------------------
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
[GitHub] [incubator-superset] zhaoyongjie commented on pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#issuecomment-691368582
@ktmud
### for columns comment:
I plan to add a comment label closed to each column, similar to the current index icon, and then when there is a comment in this column, the "Comment ICON" will be displayed and the comment will be displayed in the popover/modal
### for table comment:
Similar to the column display. Add a table comment icon beside the table name text
----------------------------------------------------------------
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
[GitHub] [incubator-superset] zhaoyongjie closed pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
zhaoyongjie closed pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844
----------------------------------------------------------------
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
[GitHub] [incubator-superset] ktmud edited a comment on pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#issuecomment-691343783
----------------------------------------------------------------
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
[GitHub] [incubator-superset] zhaoyongjie commented on pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#issuecomment-691368582
----------------------------------------------------------------
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
[GitHub] [incubator-superset] ktmud commented on pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#issuecomment-691343783
----------------------------------------------------------------
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
[GitHub] [incubator-superset] ktmud commented on pull request #10844: Feat: Adding table comment and columns comment for SQLLab
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #10844:
URL: https://github.com/apache/incubator-superset/pull/10844#issuecomment-691343783
Do you mind fixing the CI (simply run `black superset/`) and sharing a little bit how may this be used in the UI or API?
----------------------------------------------------------------
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