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 2019/05/07 17:13:47 UTC

[GitHub] [incubator-superset] DiggidyDave commented on a change in pull request #7449: Only allow GET method for explore_json by feature flag

DiggidyDave commented on a change in pull request #7449: Only allow GET method for explore_json by feature flag
URL: https://github.com/apache/incubator-superset/pull/7449#discussion_r281735523
 
 

 ##########
 File path: superset/views/core.py
 ##########
 @@ -1238,9 +1239,12 @@ def explore_json(self, datasource_type=None, datasource_id=None):
         requests that GETs or POSTs a form_data.
 
         `self.generate_json` receives this input and returns different
-        payloads based on the request args in the first block
+        payloads based on the request args in the first block"""
 
-        TODO: break into one endpoint for each return shape"""
+        if request.method != 'POST' and not is_feature_enabled('CLIENT_CACHE'):
 
 Review comment:
   I think we should avoid coupling the two questions of:
   - is CLIENT_CACHE enabled and
   - does explore_json support GET
   
   The GET method is an established part of the API here, so its removal constitutes a breaking/non-back-compat change and should be evaluated independently of the cache feature and should include a deprecation process IMO.  I also think the `@expose` configuration should match the behavior.
   
   Consider:
   - _deprecating_ GET for all users
   - adding a feature flag to turn off GET immediately
   - make the `@expose` call `methods` param use a computed value that is something along the lines of the func below...
   
   ```
   if is_feature_enabled('CLIENT_CACHE') and is_feature_enabled('DISABLE_EXPLORE_JSON_GET'):
     raise Exception('can't have both of these....')
   elif is_feature_enabled('DISABLE_EXPLORE_JSON_GET'):
     return ['POST']
   else:
     return ['POST', 'GET']
   ```

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