Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🚨🚨Source Xero: Temporarily remove OAuth support due to user limit restrictions #42128

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

btkcodedev
Copy link
Collaborator

Closes #42126

What

OAuth support for the source-Xero connector has been removed due to restrictions on the number of OAuth users allowed for the Airbyte application.

How

Removed the OAuth authenticator, retained only the bearer authenticator, and released a new version with a breaking change.

Review guide

  1. manifest.yaml
  2. metadata.yaml
  3. xero-migrations.md

User Impact

*Breaking Change

@btkcodedev btkcodedev self-assigned this Jul 20, 2024
Copy link

vercel bot commented Jul 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview Jul 20, 2024 1:20am

@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/xero labels Jul 20, 2024
@octavia-squidington-iv octavia-squidington-iv requested a review from a team July 20, 2024 00:53
@btkcodedev btkcodedev changed the title fix: comment OAuth support due to user restriction Source Xero: Temporarily remove OAuth support due to user limit restrictions Jul 20, 2024
@btkcodedev btkcodedev changed the title Source Xero: Temporarily remove OAuth support due to user limit restrictions 🚨🚨Source Xero: Temporarily remove OAuth support due to user limit restrictions Jul 20, 2024
@btkcodedev
Copy link
Collaborator Author

btkcodedev commented Jul 20, 2024

⚠️ Spec will still show to select auth_type, that part is commented and untouched for future use

Copy link
Contributor

@natikgadzhi natikgadzhi left a comment

Choose a reason for hiding this comment

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

Mostly great! Let's figure out wording, see if tests pass, and ship.

@@ -7,7 +7,7 @@ acceptance_tests:
tests:
- spec_path: "source_xero/spec.yaml"
backward_compatibility_tests_config:
disable_for_version: "0.2.5"
disable_for_version: "2.0.0" # Disabling Oauth support due to limit restriction.
Copy link
Contributor

Choose a reason for hiding this comment

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

I have no idea if this should be 3.0 or 2.0, but hope this works ;)

remoteRegistries:
pypi:
enabled: true
packageName: airbyte-source-xero
releases:
breakingChanges:
3.0.0:
upgradeDeadline: "2024-08-30"
Copy link
Contributor

Choose a reason for hiding this comment

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

Generous, and alright with me.

3.0.0:
upgradeDeadline: "2024-08-30"
message:
Due to limitations on the number of OAuth users permitted for the Airbyte application, OAuth support has been disabled.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know the right wording, but want to make this look nicer. /cc @katmarkham

@abedzantout
Copy link
Contributor

abedzantout commented Jul 22, 2024

Hey both, just saw this.

I agree that OAuth is limited to one application only, but having only bearer strategy beats the purpose of consistent synchronization - since you could only do it once and then you'll need to generate a refresh token (not supported yet with the bearer token, at least last I checked)

Until we figure that piece out, the xero connector will be limited to one sync that should be triggered manually after generating a new token

@natikgadzhi
Copy link
Contributor

Agreed. We're working on allowing full OAuth flow in Builder, but it's not yet ready, and if Xero has single use / refresh tokens that won't refresh if we provide the token in config, that's not great 👀

@abedzantout
Copy link
Contributor

Yes, happy to contribute on this if needed!

@natikgadzhi
Copy link
Contributor

So, wait up, what's stopping us from using OAuth authenticator and setting up correct refresh token paths? /cc @btkcodedev

@btkcodedev
Copy link
Collaborator Author

Already xero has bearer and oauth strategies,
If any new partnership needs breaking change, we may want to switch back to bearer strategy only.
Isn't it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/xero
Projects
None yet
Development

Successfully merging this pull request may close these issues.

source-xero: enable on Cloud and disable OAuth.
4 participants