Skip to content

refactor(db-engine-specs): use standard OAuth 2.0 params in base class - #37010

Merged
EnxDev merged 2 commits into
masterfrom
enxdev/chore/db-engine-spec
Jan 9, 2026
Merged

refactor(db-engine-specs): use standard OAuth 2.0 params in base class#37010
EnxDev merged 2 commits into
masterfrom
enxdev/chore/db-engine-spec

Conversation

@EnxDev

@EnxDev EnxDev commented Jan 9, 2026

Copy link
Copy Markdown
Contributor

SUMMARY

This PR refactors the OAuth 2.0 authorization flow to follow a cleaner architectural pattern.

Previously, BaseEngineSpec.get_oauth2_authorization_uri() included Google-specific OAuth parameters (prompt=consent, access_type=offline, include_granted_scopes=false`) directly in the base class. While this worked for Google Sheets, it's not the right design since these are provider-specific parameters rather than standard OAuth 2.0.

The refactor moves to a cleaner separation of concerns:

Base class: Uses only standard OAuth 2.0 parameters (scope, response_type, state, redirect_uri, client_id)
Google Sheets: Overrides the method to add Google-specific parameters needed for proper token refresh
This makes the codebase more maintainable and allows future OAuth-enabled database connections to work without needing to handle Google-specific quirks.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

  1. Verify Google Sheets OAuth still works correctly
  2. Run the unit tests: pytest tests/unit_tests/db_engine_specs/test_base.py::test_get_oauth2_authorization_uri_standard_params tests/unit_tests/db_engine_specs/test_gsheets.py::test_get_oauth2_authorization_uri

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • 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

@netlify

netlify Bot commented Jan 9, 2026

Copy link
Copy Markdown

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 5712265
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6960d9d8511790000840d941
😎 Deploy Preview https://deploy-preview-37010--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@EnxDev
EnxDev requested a review from betodealmeida January 9, 2026 13:29
@EnxDev
EnxDev marked this pull request as ready for review January 9, 2026 13:29
@EnxDev EnxDev added 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR and removed review:draft labels Jan 9, 2026
@dosubot dosubot Bot added the data:connect Namespace | Anything related to db connections / integrations label Jan 9, 2026
@github-actions github-actions Bot added 🎪 2698a98 🚦 building 🎪 ⌛ 48h Environment expires after 48 hours (default) and removed 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR labels Jan 9, 2026
@github-actions

github-actions Bot commented Jan 9, 2026

Copy link
Copy Markdown
Contributor

🎪 Showtime is building environment on GHA for 2698a98

@github-actions

github-actions Bot commented Jan 9, 2026

Copy link
Copy Markdown
Contributor

🎪 Showtime deployed environment on GHA for 2698a98

Environment: http://35.90.48.240:8080 (admin/admin)
Lifetime: 48h auto-cleanup
Updates: New commits create fresh environments automatically

@bito-code-review

bito-code-review Bot commented Jan 9, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #2a8467

Actionable Suggestions - 0
Review Details
  • Files reviewed - 5 · Commit Range: 5712265..2698a98
    • superset/db_engine_specs/base.py
    • superset/db_engine_specs/gsheets.py
    • tests/unit_tests/db_engine_specs/test_base.py
    • tests/unit_tests/db_engine_specs/test_gsheets.py
    • tests/unit_tests/sql_lab_test.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@betodealmeida betodealmeida left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks for the fix!

@EnxDev
EnxDev merged commit ea90d1f into master Jan 9, 2026
75 of 124 checks passed
@EnxDev
EnxDev deleted the enxdev/chore/db-engine-spec branch January 9, 2026 16:44
aminghadersohi pushed a commit to aminghadersohi/superset that referenced this pull request Jan 17, 2026
jesperct pushed a commit to jesperct/superset that referenced this pull request Jan 19, 2026
aminghadersohi pushed a commit to aminghadersohi/superset that referenced this pull request Jan 24, 2026
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
@mistercrunch mistercrunch removed the 🎪 ⌛ 48h Environment expires after 48 hours (default) label Jun 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data:connect Namespace | Anything related to db connections / integrations size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants