betodealmeida opened a new pull request, #30067: URL: https://github.com/apache/superset/pull/30067
<!--- Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/ Example: fix(dashboard): load charts correctly --> ### SUMMARY <!--- Describe the change below, including rationale and design decisions --> [SIP-99](https://github.com/apache/superset/issues/25048) introduced in https://github.com/apache/superset/pull/24969 the amazing `@transaction` decorator, which transparently manages transactions in Superset in order to avoid eager commits. [SIP-85](https://github.com/apache/superset/issues/20300) introduced a mechanism for authenticating to databases via OAuth2, using exceptions to signalize to the frontend from deep in the backend that the user needs to authenticate by following a link. These exceptions are bening, and carry in their `extra` payload (see [SIP-40](https://github.com/apache/superset/issues/9194)) the URL used for authentication, among others. Because of this, when adding a database that has OAuth2 enabled we need to allow certain exceptions to still commit, while raising them. This allows the database to be created succesfully, while signalizing to the user that they still need to authenticate. This is needed because OAuth2 tokens are stored associated with a database ID, so it must exist a priori. This PR changes the `@transaction` decorator to allow an optional group of exceptions that should trigger a commit before being re-raised. It's part of a bigger work to add support for OAuth2 to Snowflake, still in progress but already tested end-to-end. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF <!--- Skip this if not applicable --> N/A ### TESTING INSTRUCTIONS <!--- Required! What steps can be taken to manually verify the changes? --> Added unit tests for the `@transaction` decorator, covering the old use cases and the new one. ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
