[GitHub] john-bodley opened a new pull request #4500: [payload] Fixing regression introduced in ##4396

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

john-bodley opened a new pull request #4500: [payload] Fixing regression introduced in ##4396
URL: https://github.com/apache/incubator-superset/pull/4500
 
 
   This PR fixes a regression introduced in https://github.com/apache/incubator-superset/pull/4396 which returned the error `No data` even when the query was unsuccessful for other reasons. This PR also refactors the `No data` logic (previously defined in two places) and also registered that the data was loaded if the query didn't fail. Note it seems like `BaseViz.get_df` may never throw an exception give [this](https://github.com/apache/incubator-superset/blob/master/superset/connectors/sqla/models.py#L689) (and thus I'm uncertain whether [this](https://github.com/apache/incubator-superset/blob/master/superset/viz.py#L332) logic is needed) hence the additional check that the query succeeded is necessary to confirm that the data was loaded.
   
   @mistercrunch from our offline conversation I found this difficult to write specific test cases, i.e., we discovered this regression in Presto when the underlying table metadata had changed either a column or the table no-longer existed. When I tried to mock this behavior, I was able to exercise the issue as in SQLite the inconsistency was discovered whilst the query was being compiled rather than during query execution, i.e., 
   
       def test_payload_propagate_exceptions(self):
           self.login('admin')
   
           # Register table which does not exist in the underlying database.
           tbl_id = SqlaTable.import_obj(
               self.create_table(
                   'non_existent',
                   id=10006,
                   cols_names=['col1'],
                   metric_names=['m1'],
               ),
           )
   
           slc_id = models.Slice.import_obj(
               self.create_slice(
                   'dummy',
                   id=10012,
                   table_name='non_existent',
               ),
               None,
           )
   
           slc = self.get_slice(slc_id)
   
           url = self.get_slice(slc_id).get_explore_url(
               base_url='/superset/explore_json',
               overrides={'entity': 'col1', 'size': 'm1', 'x': 'm1', 'y': 'm1'},
           )
   
           data = self.get_json_resp(url, raise_on_error=False) 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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