Skip to content

feat(FS-1981): connector oauth refresh#708

Merged
jordan-homan merged 2 commits into
mainfrom
oauth_refresh
May 14, 2026
Merged

feat(FS-1981): connector oauth refresh#708
jordan-homan merged 2 commits into
mainfrom
oauth_refresh

Conversation

@jordan-homan
Copy link
Copy Markdown
Contributor

@jordan-homan jordan-homan commented May 14, 2026

Note

Low Risk
Low risk schema-only change: adds new optional config fields and updates documentation/versioning, with no runtime token refresh behavior added here.

Overview
Bumps the library version to 1.6.1 and documents the change in CHANGELOG.md.

Adds an optional refresh_token field to GoogleDriveAccessConfig, GcsAccessConfig, OnedriveAccessConfig, and OutlookAccessConfig so the platform can persist refresh tokens alongside access tokens (and removes wording that these connectors do not refresh tokens).

Reviewed by Cursor Bugbot for commit 3130452. Bugbot is set up for automated code reviews on this repo. Configure here.

@potter-potter
Copy link
Copy Markdown
Contributor

@claude please review

@claude
Copy link
Copy Markdown

claude Bot commented May 14, 2026

Claude finished @potter-potter's task —— View job


Code Review for PR #708

Todo List:

  • Read and analyze CHANGELOG.md changes
  • Review version bump in version.py
  • Analyze GCS connector changes
  • Review Google Drive connector changes
  • Examine OneDrive connector modifications
  • Check Outlook connector updates
  • Provide comprehensive review feedback

Review Summary

This PR adds optional refresh_token fields to OAuth-based connectors and updates the version to 1.6.1. The changes are well-structured and low-risk, focusing on schema additions without runtime behavior changes.

Strengths

  1. Consistent Implementation: The refresh_token field is added consistently across all OAuth connectors (Google Drive, GCS, OneDrive, Outlook) with identical field descriptions and patterns.

  2. Good Documentation: Each field includes clear descriptions explaining the purpose and lifecycle of refresh tokens:

    refresh_token: Optional[str] = Field(
        default=None,
        description="OAuth 2.0 refresh token for obtaining new access tokens. "
        "Long-lived; used by the platform to refresh expired access tokens "
        "before each job run.",
    )
  3. Proper Versioning: Version bumped to 1.6.1 in __version__.py:1 and documented in CHANGELOG.md:1-5 with a clear enhancement description.

  4. Security-Conscious: Fields are properly typed as Optional[str] with default=None, ensuring no breaking changes for existing configurations.

📝 Minor Observations

  1. Field Positioning: The refresh_token field is consistently placed after existing auth fields, maintaining logical grouping in all AccessConfig classes.

  2. No Runtime Logic: As noted in the PR description, this is purely a schema change - no token refresh logic is implemented here, which keeps the change scope minimal.

  3. Consistent with Existing Patterns: The implementation follows the same pattern as the existing oauth_token field that was added in previous versions.

🔍 Technical Details

  • GCS (gcs.py:75-80): Added to GcsAccessConfig alongside existing service_account_key and token fields
  • Google Drive (google_drive.py:72-77): Added to GoogleDriveAccessConfig after oauth_token field
  • OneDrive (onedrive.py:66-71): Added to OnedriveAccessConfig with consistent description
  • Outlook (outlook.py:51-56): Added to OutlookAccessConfig following the same pattern

Approval Recommendation

This PR is ready for approval. It's a clean, schema-only addition that:

  • ✅ Maintains backward compatibility
  • ✅ Follows consistent patterns across connectors
  • ✅ Has proper documentation and versioning
  • ✅ Poses minimal risk (no runtime behavior changes)
  • ✅ Enables future platform functionality for OAuth refresh

The implementation is straightforward and well-executed. No changes needed.


Copy link
Copy Markdown
Contributor

@potter-potter potter-potter left a comment

Choose a reason for hiding this comment

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

LG

@jordan-homan jordan-homan merged commit 18f8970 into main May 14, 2026
38 of 39 checks passed
@jordan-homan jordan-homan deleted the oauth_refresh branch May 14, 2026 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants