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/01/13 22:16:05 UTC

[GitHub] [incubator-superset] mistercrunch opened a new pull request #8960: fix: shut off unneeded endpoints

mistercrunch opened a new pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960
 
 
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r368236716
 
 

 ##########
 File path: superset/app.py
 ##########
 @@ -324,23 +323,11 @@ def init_views(self) -> None:
             category_label=__("Sources"),
             category_icon="fa-wrench",
         )
-        appbuilder.add_link(
-            "Tables",
-            label=__("Tables"),
-            href="/tablemodelview/list/?_flt_1_is_sqllab_view=y",
-            icon="fa-table",
-            category="Sources",
-            category_label=__("Sources"),
-            category_icon="fa-table",
-        )
 
         #
         # Conditionally setup log views
         #
-        if (
-            not self.config["FAB_ADD_SECURITY_VIEWS"] is False
-            or self.config["SUPERSET_LOG_VIEW"] is False
-        ):
+        if self.config["FAB_ADD_SECURITY_VIEWS"] and self.config["SUPERSET_LOG_VIEW"]:
 
 Review comment:
   No logic change, just simplifying the logic

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] codecov-io commented on issue #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#issuecomment-575924145
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8960?src=pr&el=h1) Report
   > Merging [#8960](https://codecov.io/gh/apache/incubator-superset/pull/8960?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/2fc5fd4f2921f24e5064bf54561eff87e754409a?src=pr&el=desc) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/8960/graphs/tree.svg?width=650&token=KsB0fHcx6l&height=150&src=pr)](https://codecov.io/gh/apache/incubator-superset/pull/8960?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master    #8960   +/-   ##
   =======================================
     Coverage   59.16%   59.16%           
   =======================================
     Files         367      367           
     Lines       11679    11679           
     Branches     2862     2862           
   =======================================
     Hits         6910     6910           
     Misses       4590     4590           
     Partials      179      179
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/8960?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...rset/assets/src/dashboard/actions/sliceEntities.js](https://codecov.io/gh/apache/incubator-superset/pull/8960/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9kYXNoYm9hcmQvYWN0aW9ucy9zbGljZUVudGl0aWVzLmpz) | `9.67% <ø> (ø)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8960?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8960?src=pr&el=footer). Last update [2fc5fd4...06c58ce](https://codecov.io/gh/apache/incubator-superset/pull/8960?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r368236698
 
 

 ##########
 File path: superset/app.py
 ##########
 @@ -218,6 +211,16 @@ def init_views(self) -> None:
             category_label=__("Sources"),
             category_icon="fa-database",
         )
+        appbuilder.add_link(
 
 Review comment:
   side mission: fix the menu ordering

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r368236899
 
 

 ##########
 File path: superset/security/manager.py
 ##########
 @@ -76,6 +78,13 @@ def __init__(self, **kwargs):
 PermissionViewModelView.list_widget = SupersetSecurityListWidget
 PermissionModelView.list_widget = SupersetSecurityListWidget
 
+# Limiting routes on FAB model views
+UserModelView.include_route_methods = CRUD_ROUTE_METHODS | {"userinfo"}
 
 Review comment:
   This is referenced in top menu, let's sort this out in another PR that may remove that functionality. Trying to limit this PR to "endpoints we have high confidence that we don't need"

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] craig-rueda commented on a change in pull request #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r369234581
 
 

 ##########
 File path: superset/security/manager.py
 ##########
 @@ -76,8 +78,17 @@ def __init__(self, **kwargs):
 PermissionViewModelView.list_widget = SupersetSecurityListWidget
 PermissionModelView.list_widget = SupersetSecurityListWidget
 
+# Limiting routes on FAB model views
+UserModelView.include_route_methods = CRUD_ROUTE_METHODS | {"userinfo"}
+RoleModelView.include_route_methods = CRUD_ROUTE_METHODS
+PermissionViewModelView.include_route_methods = {"list"}
 
 Review comment:
   Dry these up? You added a constant above ^^

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r369398242
 
 

 ##########
 File path: superset/app.py
 ##########
 @@ -324,23 +323,11 @@ def init_views(self) -> None:
             category_label=__("Sources"),
             category_icon="fa-wrench",
         )
-        appbuilder.add_link(
-            "Tables",
-            label=__("Tables"),
-            href="/tablemodelview/list/?_flt_1_is_sqllab_view=y",
-            icon="fa-table",
-            category="Sources",
-            category_label=__("Sources"),
-            category_icon="fa-table",
-        )
 
         #
         # Conditionally setup log views
         #
-        if (
-            not self.config["FAB_ADD_SECURITY_VIEWS"] is False
-            or self.config["SUPERSET_LOG_VIEW"] is False
-        ):
+        if self.config["FAB_ADD_SECURITY_VIEWS"] and self.config["SUPERSET_LOG_VIEW"]:
 
 Review comment:
   Oh right. Well then I'm fixing the logic. It's not super clear reading the code that the `or` operator precedes `not`.
   
   But I'm thinking that if either of these settings are moved away from their default (of `True`), we should not show the log view. 

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] craig-rueda commented on a change in pull request #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r369241198
 
 

 ##########
 File path: superset/app.py
 ##########
 @@ -324,23 +323,11 @@ def init_views(self) -> None:
             category_label=__("Sources"),
             category_icon="fa-wrench",
         )
-        appbuilder.add_link(
-            "Tables",
-            label=__("Tables"),
-            href="/tablemodelview/list/?_flt_1_is_sqllab_view=y",
-            icon="fa-table",
-            category="Sources",
-            category_label=__("Sources"),
-            category_icon="fa-table",
-        )
 
         #
         # Conditionally setup log views
         #
-        if (
-            not self.config["FAB_ADD_SECURITY_VIEWS"] is False
-            or self.config["SUPERSET_LOG_VIEW"] is False
-        ):
+        if self.config["FAB_ADD_SECURITY_VIEWS"] and self.config["SUPERSET_LOG_VIEW"]:
 
 Review comment:
   Logic is not the same as it was. If this was the previous logic, then the contrapositive would be correct: 
   ```
   not (a is False or b is False) == a and b
   ```
   but the above statement lacks the inner parens:
   ```
   # The not negates the result of a is False
   not a is False or b is False
   ```

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] dpgaspar commented on a change in pull request #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r368982788
 
 

 ##########
 File path: requirements.txt
 ##########
 @@ -22,7 +22,7 @@ croniter==0.3.30
 cryptography==2.7
 decorator==4.4.0          # via retry
 defusedxml==0.6.0         # via python3-openid
-flask-appbuilder==2.2.1
+flask-appbuilder==2.2.2rc1
 
 Review comment:
   The latest rc is rc3. When no more features are needed to support this I'll release 2.2.2
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] john-bodley edited a comment on issue #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
john-bodley edited a comment on issue #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#issuecomment-579390650
 
 
   @mistercrunch we use the `/sqlmetricinlineview/api/*` and `/tablecolumninlineview/api/*` routes syncing metadata.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r366058418
 
 

 ##########
 File path: superset/app.py
 ##########
 @@ -261,14 +255,11 @@ def init_views(self) -> None:
         appbuilder.add_view_no_menu(Dashboard)
         appbuilder.add_view_no_menu(DashboardAddView)
         appbuilder.add_view_no_menu(DashboardModelViewAsync)
-        appbuilder.add_view_no_menu(DatabaseAsync)
-        appbuilder.add_view_no_menu(DatabaseTablesAsync)
 
 Review comment:
   These 2 are unused now...

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 issue #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on issue #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#issuecomment-577335778
 
 
   @etr2460 @graceguo-supercat , we'd like to move forward and merge this, let us know if you use other endpoints that we can open up for the time being

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r368237042
 
 

 ##########
 File path: superset/views/dashboard/views.py
 ##########
 @@ -117,18 +121,3 @@ class DashboardModelViewAsync(DashboardModelView):  # pylint: disable=too-many-a
         "creator": _("Creator"),
         "modified": _("Modified"),
     }
-
-
-class DashboardAddView(DashboardModelView):  # pylint: disable=too-many-ancestors
 
 Review comment:
   Not needed anymore now that we have `ModelRestApi`

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] willbarrett commented on a change in pull request #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
willbarrett commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r367018065
 
 

 ##########
 File path: superset/views/log/views.py
 ##########
 @@ -24,3 +24,4 @@
 
 class LogModelView(LogMixin, SupersetModelView):  # pylint: disable=too-many-ancestors
     datamodel = SQLAInterface(models.Log)
+    include_route_methods = {"list"}
 
 Review comment:
   Unless this is 100% required for application functionality, I would recommend against exposing this information.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r366058558
 
 

 ##########
 File path: superset/app.py
 ##########
 @@ -261,14 +255,11 @@ def init_views(self) -> None:
         appbuilder.add_view_no_menu(Dashboard)
         appbuilder.add_view_no_menu(DashboardAddView)
         appbuilder.add_view_no_menu(DashboardModelViewAsync)
-        appbuilder.add_view_no_menu(DatabaseAsync)
-        appbuilder.add_view_no_menu(DatabaseTablesAsync)
         appbuilder.add_view_no_menu(Datasource)
         appbuilder.add_view_no_menu(KV)
         appbuilder.add_view_no_menu(R)
         appbuilder.add_view_no_menu(SavedQueryView)
         appbuilder.add_view_no_menu(SavedQueryViewApi)
-        appbuilder.add_view_no_menu(SliceAddView)
 
 Review comment:
   Merged with `SliceAsync` to simplify

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] dpgaspar commented on a change in pull request #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r366909543
 
 

 ##########
 File path: superset/security/manager.py
 ##########
 @@ -76,6 +78,13 @@ def __init__(self, **kwargs):
 PermissionViewModelView.list_widget = SupersetSecurityListWidget
 PermissionModelView.list_widget = SupersetSecurityListWidget
 
+# Limiting routes on FAB model views
+UserModelView.include_route_methods = CRUD_ROUTE_METHODS | {"userinfo"}
 
 Review comment:
   Evaluate including `show`, contains helpful admin/audit info about the user

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] mistercrunch edited a comment on issue #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
mistercrunch edited a comment on issue #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#issuecomment-577335778
 
 
   @etr2460 @graceguo-supercat , we'd like to move forward and merge this, let us know if you use other endpoints that we can open up for the time being.
   
   If you share a list of endpoints you use we can take that into account as we do change management in this area.

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] dpgaspar commented on a change in pull request #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r368985172
 
 

 ##########
 File path: tests/core_tests.py
 ##########
 @@ -345,7 +345,7 @@ def test_add_slice(self):
     def test_get_user_slices(self):
         self.login(username="admin")
         userid = security_manager.find_user("admin").id
-        url = "/sliceaddview/api/read?_flt_0_created_by={}".format(userid)
+        url = "/sliceasync/api/read?_flt_0_created_by={}".format(userid)
 
 Review comment:
   nit: Take the chance to use f strings

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] craig-rueda commented on a change in pull request #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r369681234
 
 

 ##########
 File path: superset/app.py
 ##########
 @@ -324,23 +323,11 @@ def init_views(self) -> None:
             category_label=__("Sources"),
             category_icon="fa-wrench",
         )
-        appbuilder.add_link(
-            "Tables",
-            label=__("Tables"),
-            href="/tablemodelview/list/?_flt_1_is_sqllab_view=y",
-            icon="fa-table",
-            category="Sources",
-            category_label=__("Sources"),
-            category_icon="fa-table",
-        )
 
         #
         # Conditionally setup log views
         #
-        if (
-            not self.config["FAB_ADD_SECURITY_VIEWS"] is False
-            or self.config["SUPERSET_LOG_VIEW"] is False
-        ):
+        if self.config["FAB_ADD_SECURITY_VIEWS"] and self.config["SUPERSET_LOG_VIEW"]:
 
 Review comment:
   I totally agree that this logical statement was funky. Took me a while to unravel it myself. If you're good with just simplifying it and taking whatever behavior changes that come out of it, then I'm good with that too.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r368236998
 
 

 ##########
 File path: superset/views/core.py
 ##########
 @@ -2984,16 +2983,3 @@ def apply_http_headers(response: Response):
         if k not in response.headers:
             response.headers[k] = v
     return response
-
-
-@app.route('/<regex("panoramix\/.*"):url>')
 
 Review comment:
   Killing redirects for things that would have been bookmarked 4+ years ago

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] willbarrett commented on a change in pull request #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
willbarrett commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r367018419
 
 

 ##########
 File path: superset/security/manager.py
 ##########
 @@ -76,6 +78,13 @@ def __init__(self, **kwargs):
 PermissionViewModelView.list_widget = SupersetSecurityListWidget
 PermissionModelView.list_widget = SupersetSecurityListWidget
 
+# Limiting routes on FAB model views
+UserModelView.include_route_methods = CRUD_ROUTE_METHODS | {"userinfo"}
 
 Review comment:
   Same here. I'd recommend restricting this info unless it's required for application functionality.

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] codecov-io edited a comment on issue #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#issuecomment-575924145
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8960?src=pr&el=h1) Report
   > Merging [#8960](https://codecov.io/gh/apache/incubator-superset/pull/8960?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/e46ff239afc79af5753cfe82779fa87796f46a27?src=pr&el=desc) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/8960/graphs/tree.svg?width=650&token=KsB0fHcx6l&height=150&src=pr)](https://codecov.io/gh/apache/incubator-superset/pull/8960?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master    #8960   +/-   ##
   =======================================
     Coverage   59.16%   59.16%           
   =======================================
     Files         367      367           
     Lines       11679    11679           
     Branches     2862     2862           
   =======================================
     Hits         6910     6910           
     Misses       4590     4590           
     Partials      179      179
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/8960?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...rset/assets/src/dashboard/actions/sliceEntities.js](https://codecov.io/gh/apache/incubator-superset/pull/8960/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9kYXNoYm9hcmQvYWN0aW9ucy9zbGljZUVudGl0aWVzLmpz) | `9.67% <ø> (ø)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8960?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8960?src=pr&el=footer). Last update [e46ff23...c868620](https://codecov.io/gh/apache/incubator-superset/pull/8960?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] request-info[bot] commented on issue #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
request-info[bot] commented on issue #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#issuecomment-573898597
 
 
   We would appreciate it if you could provide us with more info about this issue/pr! Please do not leave the `title` or `description` empty.
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
mistercrunch merged pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960
 
 
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r367243043
 
 

 ##########
 File path: superset/views/log/views.py
 ##########
 @@ -24,3 +24,4 @@
 
 class LogModelView(LogMixin, SupersetModelView):  # pylint: disable=too-many-ancestors
     datamodel = SQLAInterface(models.Log)
+    include_route_methods = {"list"}
 
 Review comment:
   I think what makes sense is either `{"list", "show"}` or nothing here. I was thinking about leaving it for backwards compatibility. I could put it behind a config flag too, let me do that.

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] villebro commented on a change in pull request #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r369410454
 
 

 ##########
 File path: superset/app.py
 ##########
 @@ -324,23 +323,11 @@ def init_views(self) -> None:
             category_label=__("Sources"),
             category_icon="fa-wrench",
         )
-        appbuilder.add_link(
-            "Tables",
-            label=__("Tables"),
-            href="/tablemodelview/list/?_flt_1_is_sqllab_view=y",
-            icon="fa-table",
-            category="Sources",
-            category_label=__("Sources"),
-            category_icon="fa-table",
-        )
 
         #
         # Conditionally setup log views
         #
-        if (
-            not self.config["FAB_ADD_SECURITY_VIEWS"] is False
-            or self.config["SUPERSET_LOG_VIEW"] is False
-        ):
+        if self.config["FAB_ADD_SECURITY_VIEWS"] and self.config["SUPERSET_LOG_VIEW"]:
 
 Review comment:
   I think the original slightly weird double negative notation is a consequence of originally having assumed that config parameters weren't always set. This has changed with the adoption of square brackets when retrieving config params over `get()`. I think it makes sense to think critically of what the behavour should be instead of persisting the existing logic as-is.

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] etr2460 commented on issue #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
etr2460 commented on issue #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#issuecomment-575275438
 
 
   @graceguo-supercat do you know if we use any of these endpoints for programmatic chart/dashboard generation?

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] dpgaspar commented on a change in pull request #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#discussion_r366889407
 
 

 ##########
 File path: superset/views/log/views.py
 ##########
 @@ -24,3 +24,4 @@
 
 class LogModelView(LogMixin, SupersetModelView):  # pylint: disable=too-many-ancestors
     datamodel = SQLAInterface(models.Log)
+    include_route_methods = {"list"}
 
 Review comment:
   Consider adding "show" also, since the fields retrieved by list are not very useful, it's important to access the log detail

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-superset] john-bodley commented on issue #8960: fix: shut off unneeded endpoints

Posted by GitBox <gi...@apache.org>.
john-bodley commented on issue #8960: fix: shut off unneeded endpoints
URL: https://github.com/apache/incubator-superset/pull/8960#issuecomment-579390650
 
 
   @mistercrunch we use the `/sqlmetricinlineview/api/` and `/tablecolumninlineview/api` routes syncing metadata.

----------------------------------------------------------------
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


With regards,
Apache Git Services

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