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.

Sign up for GitHub

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

Jump to bottom

Sources Jira, Github: update CDK authenticator #38341

Merged
merged 8 commits into from
May 22, 2024
Merged

Sources Jira, Github: update CDK authenticator #38341

merged 8 commits into from
May 22, 2024

Conversation

ChristoGrab
Copy link
Contributor

@ChristoGrab ChristoGrab commented May 20, 2024 •
edited
Loading

What

Migrates the authenticators for sources Jira and Github from the deprecated auth package to the current requests_native_auth package. Batching these as the three connectors that did not complain after the update.

User Impact

No impact

Can this PR be safely reverted and rolled back?

  • [✓ ] YES 💚
  • NO ❌

@vercel Vercel
Copy link

vercel bot commented May 20, 2024 •
edited
Loading

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

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 22, 2024 2:57pm

@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label May 20, 2024
@ChristoGrab ChristoGrab marked this pull request as ready for review May 20, 2024 13:45
@aldogonzalez8
Copy link
Contributor

This looks good to me, how did you test authentication changes for these 3 sources?

@ChristoGrab
Copy link
Contributor Author

ChristoGrab commented May 20, 2024 •
edited
Loading

This looks good to me, how did you test authentication changes for these 3 sources?

@aldogonzalez8 Glad you asked! I was going to say that for a small change like this it felt like successfully running the read interface on each connector locally and having a green test pipeline is enough. However, I'm realizing that for at least Amazon Ads there are streams we cannot populate in our sandbox account which include additional authentication steps that could cause issues that our pipeline won't catch.

I'll remove Amazon Ads from this PR for now, but I think Jira and Github should still be safe since they use the same authentication logic consistently across all streams and we are able to fetch all the expected_records in our sandbox accounts.

@ChristoGrab ChristoGrab changed the title Sources Amazon Ads, Jira, Github: update CDK authenticator Sources Jira, Github: update CDK authenticator May 21, 2024
aldogonzalez8
aldogonzalez8 approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@aldogonzalez8 aldogonzalez8 left a comment

Choose a reason for hiding this comment

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

APPROVED

@aldogonzalez8
Copy link
Contributor

Not sure if this is important but it shows two failing jobs and "Squash and merge" button looks enabled

image

@ChristoGrab ChristoGrab merged commit 77e42a7 into master May 22, 2024
33 checks passed
@ChristoGrab ChristoGrab deleted the christo/update-certified-authenticators-1 branch May 22, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aldogonzalez8 aldogonzalez8 aldogonzalez8 approved these changes

@bazarnov bazarnov bazarnov approved these changes

@cmm-airbyte cmm-airbyte Awaiting requested review from cmm-airbyte

Assignees
No one assigned
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/amazon-ads connectors/source/github connectors/source/jira
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ChristoGrab @aldogonzalez8 @bazarnov @octavia-squidington-iii