[GitHub] [superset] graceguo-supercat edited a comment on pull request #16621: fix(dashboard): label colors included in explore url

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

graceguo-supercat edited a comment on pull request #16621:
URL: https://github.com/apache/superset/pull/16621#issuecomment-915427458


   The issue for "explore URL" is **_not_** really caused by #16444. [This line](https://github.com/apache/superset/blob/3fe2e6ec731529ae228904b9b5e39baec073ae8d/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx#L286)
   ```
    <a href={this.props.exploreUrl} rel="noopener noreferrer">
   ```
   will generate a GET request when user click on it, but browser (and many companies using nginx) has limitation for length of GET request. In airbnb we had a lot of charts with very long parameters, these charts couldn't be open by GET request. #16444 added label colors into url makes things worse, but it is no the root cause. 
   
   Note, originally, there was an onClick event handler for this `View chart in explore` menu item, but it was lost since #11554. In the event handler, it generate a POST request to open the chart since GET won't work for long url.
   
   Ideally we should **avoid** opening chart by GET request. 


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

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

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