khtruong commented on a change in pull request #7770: Autocomplete in the table 
browser in SQL lab is broken - Fix part 2
URL: 
https://github.com/apache/incubator-superset/pull/7770#discussion_r297297017
 
 

 ##########
 File path: superset/assets/src/components/TableSelector.jsx
 ##########
 @@ -191,12 +199,6 @@ export default class TableSelector extends 
React.PureComponent {
       this.onChange();
     });
   }
-  addOptionIfMissing(options, value) {
 
 Review comment:
   Oh I'm sorry. I totally forgot to mention in my description. So this entire 
component is buggy and doesn't work as expected. What this function does is 
that if the option is missing, then we add it to the list with just the table 
name. I believe the original intent was to display something while we were 
fetching the tables. This led to a few bugs:
   1. We may be adding a table that no longer exists to the list, which doesn't 
make sense. So when the user selects this table, it just breaks silently 
because the table doesn't exist.
   2. Even if the table exists, we did not populate this option with the schema 
and so when we make a request to get the table, again it breaks silently 
because we get the table with just its name and no schema information.
   
   I felt that if we really wanted to display something while we fetch the 
tables, we should do it properly with tests. So I just removed the buggy code 
for now and thought we can revisit it later if that is a feature we really want.
   
   The component is still buggy but it's less buggy. I'll add this explanation 
to the description. Let me know your thoughts.

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