villebro commented on PR #27631: URL: https://github.com/apache/superset/pull/27631#issuecomment-2027427919
> 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. I'm totally ok with the hybrid approach if we make sure we're not painting ourselves into a corner with it. The reason I feel uneasy with implementing a "one client for all connections of type x" approach as the initial implementation is that I feel it's inherently atypical for this type of flow: While it may be optimal for this specific use case, I don't think it works generally for most other use cases. > 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. I understand this. Again, my reservation stems from the fact that this is not a typical OAuth2 flow, i.e. you likely can't use this for the majority of OAuth2 connectivity use cases, but the alternative I'm proposing works for this one, too. But as I said, I'm not against being able to provide static creds in the config where applicable. > 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. > (As for Snowflake, my understanding is that we can derive the OAuth2 URL from the account name, which is part of the SQLAlchemy URL.) But even if you dynamically generate the URI, you would still need to store the client creds somewhere, right? In other words, the user would still need to pass the client id and secret somehow for the backend to be able to use them. Quoting from https://docs.snowflake.com/en/user-guide/oauth-custom#request-header: <img width="861" alt="image" src="https://github.com/apache/superset/assets/33317356/ef30d7b4-f1ae-4553-b510-937753bb4515";> -- 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
