airbyte
airbyte copied to clipboard
Provide correct grant_type in Apple Ads Oauth
Based on our testing of this source, it seems like an explicit specification of grant_type: client_credentials is necessary in order not to crash the DeclarativeOauth2Authenticator.
When no grant type is specified, it expects grant type refresh token, but we have no refresh token and attempt to use client credentials
This fixes https://github.com/airbytehq/airbyte/issues/46879
What
Avoid OAuth ValueError "OauthAuthenticator needs a refresh_token parameter when grant_type is set to refresh_token when initializing a new Apple Search Ads source.
How
Explicitly specify that grant_type is client_credentials (this is already reflected in the url that we try to use)
Review guide
- Follow the installation steps for the Apple Search ads extension at main (import the yaml into your builder)
- Test the source
- This should crash with the above-mentioned error
- Apply the fix from this PR (explicitly set grant_type to client_credentials in the auth-yaml) and retest
- You should now receive data 🎉
User Impact
Now it should be possible to set up new Apple Search Ads sources again 🎉
Can this PR be safely reverted and rolled back?
- [x] YES 💚
- [ ] NO ❌
@olagjo is attempting to deploy a commit to the Airbyte Growth Team on Vercel.
A member of the Team first needs to authorize it.
Thanks for attempting to fix this @olagjo. Can you bump the version, and update the changelog to proceed with this.
PS: Since the manifest only version has been shipped to cloud, I'll suggest forking the connector in cloud builder, make your changes, test that the fix worked as expected, and contribute back to the repo. Makes sense? Let me if you need me to clarify anything. Cheers!
Hi @topefolorunso thank you for following up! I have tried to bump the version and the changelog :)
I'm also not fully following your suggestion – in order to make this fix in the first place, the first thing I did was use the manifest-only version of the source in my airbyte installation, and then tweak the one line in the manifest. The source then started working in my airbyte installation, so I decided to contribute the oneline fix upstream. Is this fundamentally different than using the cloud builder?
@olagjo Thanks for submitting this contribution, and apologies for not catching this sooner! I had to make a few additional changes to the connector's metadata to appease our CI, so for expediency I threw in your fix in a fresh PR (link above). It was merged earlier today, and you should be good to go with the new version (0.2.1). Cheers!
Thank you!