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:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to