superset-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [incubator-superset] DiggidyDave edited a comment on issue #7422: Add "validation_only" queries to the backend
Date Wed, 01 May 2019 15:40:02 GMT
DiggidyDave edited a comment on issue #7422: Add "validation_only" queries to the backend
URL: https://github.com/apache/incubator-superset/pull/7422#issuecomment-488318130
 
 
   A couple of high-level points and suggestions for consideration:
   
   1. I'm not a big database guy, but **the term 'validation query'** seems to have an [established
meaning](https://www.google.com/search?q=database+validation+query) and I fear this will be
unnecessarily overloading a term and possibly causing confusion.  Someone implementing a new
type might ask "does my database support validation queries?" and find [this comment on SO](https://stackoverflow.com/a/10684260/3407502)
and do the wrong thing.  Consider:
   
       - renaming it to something less ambiguous and more self-documenting
       - adding more comments to the base class documenting the member property and method
   
   2. Tying this to a database's built in implementation is perhaps limiting. It seem unlikely
that many (any?) new databases will supply this functionality, but if there was a configurable
"per-dialect" or "per database-type" endpoint exposing an abstract API that we define, than
lots of options could be plugged in there (including forwarding through to the provider).
Consider:
   
       - removing the validation support from `BaseEngineSpec` and having a configurable sql
validation endpoint map, which by default has one impl for Hive that forwards to the server.
 That way we reserve the right to implement the API to make this feature available to other
db types going forward. We might say _But we can always add that later!_, and that is true,
but if we stub out the interfaces and make the pattern clear and self-documenting now, then
the community might pick up the work to add verification endpoints for other database types.
The pattern we pick now is the pattern we are going to live with.

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


Mime
View raw message