🎉 Source Pipedrive: add oAuth support #6821

gaart · 2021-10-05T21:57:11Z

What

Closes #6821

Pre-merge Checklist

Community member or Airbyter

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
Credentials added to Github CI. Instructions.
/test connector=connectors/<name> command is passing.
New Connector version released on Dockerhub by running the /publish command described here

keu · 2021-10-06T19:43:47Z

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/streams.py

-        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):


looks weird, why can't we create an authenticator inherited from AuthBase? ParamAuthenticator?

@keu We can, but we're trying to add new features for OAuth without changing existing ones. Should I find another solution?

keu

see comments

keu · 2021-10-06T19:49:51Z

.github/workflows/publish-command.yml

@@ -135,6 +135,8 @@ jobs:
          MYSQL_SSH_PWD_TEST_CREDS: ${{ secrets.MYSQL_SSH_PWD_TEST_CREDS }}
          POSTHOG_TEST_CREDS: ${{ secrets.POSTHOG_TEST_CREDS }}
          PIPEDRIVE_INTEGRATION_TESTS_CREDS: ${{ secrets.PIPEDRIVE_INTEGRATION_TESTS_CREDS }}
+          PIPEDRIVE_INTEGRATION_TESTS_CREDS_OAUTH: ${{ secrets.PIPEDRIVE_INTEGRATION_TESTS_CREDS_OAUTH }}
+          PIPEDRIVE_INTEGRATION_TESTS_CREDS_OLD: ${{ secrets.PIPEDRIVE_INTEGRATION_TESTS_CREDS_OLD }}


Suggested change

PIPEDRIVE_INTEGRATION_TESTS_CREDS_OLD: ${{ secrets.PIPEDRIVE_INTEGRATION_TESTS_CREDS_OLD }}

PIPEDRIVE_INTEGRATION_TESTS_CREDS_TOKEN: ${{ secrets.PIPEDRIVE_INTEGRATION_TESTS_CREDS_TOKEN }}

@sherifnada if we going to support both,

I think _OLD is the legacy format of the config and is present to make sure backwards compat is working

keu · 2021-10-06T19:51:10Z

tools/bin/ci_credentials.sh

@@ -106,7 +106,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"


why there are 3 different configs?

1 - new style config for OAuth
2 - new style config for Token auth
3 - old style config for Token auth

gaart · 2021-10-07T16:29:55Z

/test connector=connectors/source-pipedrive

🕑 connectors/source-pipedrive https://github.com/airbytehq/airbyte/actions/runs/1317062829
❌ connectors/source-pipedrive https://github.com/airbytehq/airbyte/actions/runs/1317062829
🐛

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/spec.json

…ive/spec.json Co-authored-by: Sherif A. Nada <snadalive@gmail.com>

gaart · 2021-10-11T14:29:18Z

/test connector=connectors/source-pipedrive

🕑 connectors/source-pipedrive https://github.com/airbytehq/airbyte/actions/runs/1329381344
❌ connectors/source-pipedrive https://github.com/airbytehq/airbyte/actions/runs/1329381344
🐛

gaart · 2021-10-12T06:17:03Z

/test connector=connectors/source-pipedrive

🕑 connectors/source-pipedrive https://github.com/airbytehq/airbyte/actions/runs/1331867469
✅ connectors/source-pipedrive https://github.com/airbytehq/airbyte/actions/runs/1331867469
Python tests coverage:

	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 /actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-pipedrive/.venv/lib/python3.8/site-packages/coverage/control.py:761: CoverageWarning: No data was collected. (no-data-collected)
	 Name                           Stmts   Miss  Cover
	   self._warn("No data was collected.", slug="no-data-collected")
	 --------------------------------------------------
	 source_pipedrive/__init__.py       2      2     0%
	 source_pipedrive/source.py        36     36     0%
	 source_pipedrive/streams.py       75     75     0%
	 --------------------------------------------------
	 TOTAL                            113    113     0%
	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 /actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-pipedrive/.venv/lib/python3.8/site-packages/coverage/control.py:761: CoverageWarning: No data was collected. (no-data-collected)
	 Name                           Stmts   Miss  Cover
	   self._warn("No data was collected.", slug="no-data-collected")
	 --------------------------------------------------
	 source_pipedrive/__init__.py       2      2     0%
	 source_pipedrive/source.py        36     36     0%
	 source_pipedrive/streams.py       75     75     0%
	 --------------------------------------------------
	 TOTAL                            113    113     0%
	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                                 Stmts   Miss  Cover
	 ------------------------------------------------------------------------
	 source_acceptance_test/__init__.py                       2      0   100%
	 source_acceptance_test/base.py                          10      4    60%
	 source_acceptance_test/config.py                        74      8    89%
	 source_acceptance_test/conftest.py                     108    108     0%
	 source_acceptance_test/plugin.py                        47     47     0%
	 source_acceptance_test/tests/__init__.py                 4      0   100%
	 source_acceptance_test/tests/test_core.py              200     94    53%
	 source_acceptance_test/tests/test_full_refresh.py       18     11    39%
	 source_acceptance_test/tests/test_incremental.py        69     38    45%
	 source_acceptance_test/utils/__init__.py                 6      0   100%
	 source_acceptance_test/utils/asserts.py                 37      2    95%
	 source_acceptance_test/utils/common.py                  41     24    41%
	 source_acceptance_test/utils/compare.py                 47     20    57%
	 source_acceptance_test/utils/connector_runner.py        82     49    40%
	 source_acceptance_test/utils/json_schema_helper.py     115     14    88%
	 ------------------------------------------------------------------------
	 TOTAL                                                  860    419    51%

gaart · 2021-10-12T06:25:58Z

/publish connector=connectors/source-pipedrive

🕑 connectors/source-pipedrive https://github.com/airbytehq/airbyte/actions/runs/1331891848
✅ connectors/source-pipedrive https://github.com/airbytehq/airbyte/actions/runs/1331891848

* Add oauth support * Upd changelog * Update airbyte-integrations/connectors/source-pipedrive/source_pipedrive/spec.json Co-authored-by: Sherif A. Nada <snadalive@gmail.com> Co-authored-by: Sherif A. Nada <snadalive@gmail.com>

Add oauth support

6eed871

github-actions bot added the area/connectors Connector related issues label Oct 5, 2021

Upd changelog

9d4efb8

github-actions bot added the area/documentation Improvements or additions to documentation label Oct 5, 2021

gaart temporarily deployed to more-secrets October 5, 2021 21:59 Inactive

gaart linked an issue Oct 6, 2021 that may be closed by this pull request

Source Pipedrive: support oauth #6278

Closed

gaart requested review from eliziario, keu and vitaliizazmic October 6, 2021 06:13

keu reviewed Oct 6, 2021

View reviewed changes

keu suggested changes Oct 6, 2021

View reviewed changes

keu reviewed Oct 6, 2021

View reviewed changes

jrhizor temporarily deployed to more-secrets October 7, 2021 16:32 Inactive

gaart requested a review from keu October 7, 2021 16:37

sherifnada approved these changes Oct 7, 2021

View reviewed changes

airbyte-integrations/connectors/source-pipedrive/source_pipedrive/spec.json Outdated Show resolved Hide resolved

gaart and others added 2 commits October 11, 2021 17:27

Update airbyte-integrations/connectors/source-pipedrive/source_pipedr…

98dacf3

…ive/spec.json Co-authored-by: Sherif A. Nada <snadalive@gmail.com>

Merge branch 'master' into gaart/6278-source-pipedrive-oauth-support

04fc0d2

gaart temporarily deployed to more-secrets October 11, 2021 14:30 Inactive

jrhizor temporarily deployed to more-secrets October 11, 2021 14:31 Inactive

jrhizor temporarily deployed to more-secrets October 12, 2021 06:19 Inactive

jrhizor temporarily deployed to more-secrets October 12, 2021 06:28 Inactive

gaart merged commit 99b1e91 into master Oct 12, 2021

gaart deleted the gaart/6278-source-pipedrive-oauth-support branch October 12, 2021 06:45

jrhizor mentioned this pull request Oct 13, 2021

Bump Airbyte version from 0.30.18-alpha to 0.30.19-alpha #6991

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎉 Source Pipedrive: add oAuth support #6821

🎉 Source Pipedrive: add oAuth support #6821

gaart commented Oct 5, 2021 •

edited

Loading

keu Oct 6, 2021 •

edited

Loading

gaart Oct 7, 2021

keu left a comment

keu Oct 6, 2021

sherifnada Oct 7, 2021

gaart Oct 7, 2021

keu Oct 6, 2021

gaart Oct 7, 2021

gaart commented Oct 7, 2021 •

edited by github-actions bot

Loading

gaart commented Oct 11, 2021 •

edited by github-actions bot

Loading

gaart commented Oct 12, 2021 •

edited by github-actions bot

Loading

gaart commented Oct 12, 2021 •

edited by github-actions bot

Loading

	PIPEDRIVE_INTEGRATION_TESTS_CREDS_OLD: ${{ secrets.PIPEDRIVE_INTEGRATION_TESTS_CREDS_OLD }}
	PIPEDRIVE_INTEGRATION_TESTS_CREDS_TOKEN: ${{ secrets.PIPEDRIVE_INTEGRATION_TESTS_CREDS_TOKEN }}

🎉 Source Pipedrive: add oAuth support #6821

🎉 Source Pipedrive: add oAuth support #6821

Conversation

gaart commented Oct 5, 2021 • edited Loading

What

Pre-merge Checklist

Community member or Airbyter

Airbyter

keu Oct 6, 2021 • edited Loading

Choose a reason for hiding this comment

gaart Oct 7, 2021

Choose a reason for hiding this comment

keu left a comment

Choose a reason for hiding this comment

keu Oct 6, 2021

Choose a reason for hiding this comment

sherifnada Oct 7, 2021

Choose a reason for hiding this comment

gaart Oct 7, 2021

Choose a reason for hiding this comment

keu Oct 6, 2021

Choose a reason for hiding this comment

gaart Oct 7, 2021

Choose a reason for hiding this comment

gaart commented Oct 7, 2021 • edited by github-actions bot Loading

gaart commented Oct 11, 2021 • edited by github-actions bot Loading

gaart commented Oct 12, 2021 • edited by github-actions bot Loading

gaart commented Oct 12, 2021 • edited by github-actions bot Loading

gaart commented Oct 5, 2021 •

edited

Loading

keu Oct 6, 2021 •

edited

Loading

gaart commented Oct 7, 2021 •

edited by github-actions bot

Loading

gaart commented Oct 11, 2021 •

edited by github-actions bot

Loading

gaart commented Oct 12, 2021 •

edited by github-actions bot

Loading

gaart commented Oct 12, 2021 •

edited by github-actions bot

Loading