villebro commented on PR #27631: URL: https://github.com/apache/superset/pull/27631#issuecomment-2026298773
> what are your thoughts on introducing later the concept of the OAuth2 client, that can be associated with a database in the dropdown? And for now we'd have only the DB engine spec default client. I don't think adding the client model later would require any significant refactor, the only difference is changing the logic here: ... I feel in general these types of configurations should never require changes to `superset_config.py` + subsequent redeploys. Also, if we do end up introducing the OAuth2 client model later on to support non-cloud db OAuth2 flow, then the config based approach will anyway become redundant, and will cause both maintenance burden, deprecation and removal at some point. This is based on the assumption that the best UX for adding cloud db clients would still be through the UI, where we would have a Test button that validates the client credentials, and can e.g. render a decoded token for the user to inspect and validate claims etc. So in the interest of reducing throwaway work, maintenance burden and deprecation tasks in forthcoming Superset versions, I'd personally vote for doing the full monty here. This would mean introducing the OAuth2 client model, and making sure it works both for cloud dbs (Snowflake, Gsheets etc) and the more traditional OAuth2 based db setups, especially Trino, which I suspect is the one a lot of enterprise users are eagerly looking forward to. I can of course pitch in with the Presto/Trino logic, but it's quite trivial. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
