-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 Pipedrive: add oAuth support #6821
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,7 @@ | ||
{ | ||
"api_token": "wrong-api-token", | ||
"replication_start_date": "2021-06-01T10:10:10Z" | ||
} | ||
"authorization": { | ||
"auth_type": "Token", | ||
"api_token": "wrong-api-token" | ||
}, | ||
"replication_start_date": "2021-01-01T10:10:10Z" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
import pendulum | ||
import requests | ||
from airbyte_cdk.sources.streams.http import HttpStream | ||
from airbyte_cdk.sources.streams.http.auth import NoAuth, Oauth2Authenticator | ||
|
||
PIPEDRIVE_URL_BASE = "https://api.pipedrive.com/v1/" | ||
|
||
|
@@ -18,9 +19,13 @@ class PipedriveStream(HttpStream, ABC): | |
data_field = "data" | ||
page_size = 50 | ||
|
||
def __init__(self, api_token: str, replication_start_date: pendulum.datetime = None, **kwargs): | ||
super().__init__(**kwargs) | ||
self._api_token = api_token | ||
def __init__(self, authenticator, replication_start_date=None, **kwargs): | ||
if isinstance(authenticator, Oauth2Authenticator): | ||
super().__init__(authenticator=authenticator, **kwargs) | ||
else: | ||
super().__init__(**kwargs) | ||
self._api_token = authenticator["api_token"] | ||
|
||
self._replication_start_date = replication_start_date | ||
|
||
@property | ||
|
@@ -56,7 +61,10 @@ def request_params( | |
self, stream_state: Mapping[str, Any], stream_slice: Mapping[str, any] = None, next_page_token: Mapping[str, Any] = None | ||
) -> MutableMapping[str, Any]: | ||
next_page_token = next_page_token or {} | ||
params = {"api_token": self._api_token, "limit": self.page_size, **next_page_token} | ||
params = {"limit": self.page_size, **next_page_token} | ||
|
||
if isinstance(self.authenticator, NoAuth): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looks weird, why can't we create an authenticator inherited from AuthBase? ParamAuthenticator? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @keu We can, but we're trying to add new features for OAuth without changing existing ones. Should I find another solution? |
||
params["api_token"] = self._api_token | ||
|
||
replication_start_date = self._replication_start_date | ||
if replication_start_date: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -107,7 +107,9 @@ write_standard_creds source-paypal-transaction "$PAYPAL_TRANSACTION_CREDS" | |
write_standard_creds source-mysql "$MYSQL_SSH_KEY_TEST_CREDS" "ssh-key-config.json" | ||
write_standard_creds source-mysql "$MYSQL_SSH_PWD_TEST_CREDS" "ssh-pwd-config.json" | ||
write_standard_creds source-posthog "$POSTHOG_TEST_CREDS" | ||
write_standard_creds source-pipedrive "$PIPEDRIVE_INTEGRATION_TESTS_CREDS" | ||
write_standard_creds source-pipedrive "$PIPEDRIVE_INTEGRATION_TESTS_CREDS" "config.json" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why there are 3 different configs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1 - new style config for OAuth |
||
write_standard_creds source-pipedrive "$PIPEDRIVE_INTEGRATION_TESTS_CREDS_OAUTH" "oauth_config.json" | ||
write_standard_creds source-pipedrive "$PIPEDRIVE_INTEGRATION_TESTS_CREDS_OLD" "old_config.json" | ||
write_standard_creds source-quickbooks-singer "$QUICKBOOKS_TEST_CREDS" | ||
write_standard_creds source-recharge "$RECHARGE_INTEGRATION_TEST_CREDS" | ||
write_standard_creds source-recurly "$SOURCE_RECURLY_INTEGRATION_TEST_CREDS" | ||
|
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.
@sherifnada if we going to support both,
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.
I think
_OLD
is the legacy format of the config and is present to make sure backwards compat is workingThere 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.
Correct