-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Update CDK for source-xero #27007
Update CDK for source-xero #27007
Conversation
…irbyte into flash1293/unified-single-use
Before Merging a Connector Pull RequestWow! What a great pull request you have here! 🎉 To merge this PR, ensure the following has been done/considered for each connector added or updated:
If the checklist is complete, but the CI check is failing,
|
/test connector=connectors/source-xero local_cdk=1
Build FailedTest summary info:
|
Seems like local_cdk doesn't work for unit_tests, however I ran them locally and they worked fine:
|
/test connector=connectors/source-xero
Build PassedTest summary info:
|
@@ -83,8 +83,8 @@ def get_authenticator(config: Mapping[str, Any]) -> Mapping[str, Any]: | |||
return XeroSingleUseRefreshTokenOauth2Authenticator( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems odd that we have a custom component for that. It seems to be related to HTTP requests we are performing. Eventually, it might be interesting to increase the capabilities of OAuth in the CDK to avoid having those custom things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On comment that is outside of scope for this PR
What
Update xero connector wrt the change in #26966
How
Pass in client id and client secret directly.