betodealmeida commented on PR #27631: URL: https://github.com/apache/superset/pull/27631#issuecomment-2027332363
> I'm simply trying to bring up the fact that this solution wouldn't work in a use case that I would need it to work in if I were to use this feature. The solution I'm proposing is a hybrid `superset_config.py` (this PR) + new model (future PR), which would work for **both** of of our use cases. We'd have to introduce the new model and UI elements, but most of the logic of this PR would remain unchanged. I'm not against your proposal, on the contrary, I think it's a great idea and an elegant solution. But what I'm hearing from you is that we should go with only the new model, where the client ID/secret live in the metadata database. That solution is very suboptimal for my use case, since it would require users to create and manage their own applications. You mentioned: > This is based on the assumption that the best UX for adding cloud db clients would still be through the UI But that is definitely not true in my use case. Google Sheets is one of our most popular databases used at Preset, and it would be great if users could connect to private sheets without having to figure out how to create an OAuth2 application first. BigQuery is probably **the** most popular, and the faster our users can start exploring their data, the better. You also mentioned: > then the config based approach will anyway become redundant, and will cause both maintenance burden, deprecation and removal at some point. I don't see why the config approach would have to removed, since it complements the approach you're proposing. The default configuration for the feature is just an empty dictionary. We just need to add logic that checks the for the client information in two different places, as we already do for many things in Superset — this is IMHO the **main feature** of Superset, how it's adaptable and configurable to so many different use cases. And in fairness, I do think that if we have the hybrid approach most people will prefer to use the UI to configure OAuth2, because as you said, it's easier to update the client in a single place and changes don't require a redeploy. But that doesn't mean that it's a solution that works for everyone. (As for Snowflake, my understanding is that we can derive the OAuth2 URL from the account name, which is part of the SQLAlchemy URL.) -- 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
