From notifications-return-20010-archive-asf-public=cust-asf.ponee.io@superset.apache.org Wed May 1 15:40:04 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 2DA5E180629 for ; Wed, 1 May 2019 17:40:04 +0200 (CEST) Received: (qmail 7437 invoked by uid 500); 1 May 2019 15:40:03 -0000 Mailing-List: contact notifications-help@superset.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@superset.apache.org Delivered-To: mailing list notifications@superset.apache.org Received: (qmail 7427 invoked by uid 99); 1 May 2019 15:40:03 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 01 May 2019 15:40:03 +0000 From: GitBox To: notifications@superset.apache.org Subject: [GitHub] [incubator-superset] DiggidyDave edited a comment on issue #7422: Add "validation_only" queries to the backend Message-ID: <155672520283.19154.6023492392531021281.gitbox@gitbox.apache.org> Date: Wed, 01 May 2019 15:40:02 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit 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