🎉 Add Braintree source connector #5362

avida · 2021-08-12T07:30:38Z

What

Resolves #5236 issues

How

Describe the solution

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

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

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Secrets in the connector's spec are annotated with airbyte_secret
Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Code reviews completed
Documentation updated
- Connector's README.md
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions
Connector version bumped like described here

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

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed.

avida · 2021-08-12T07:33:10Z

/test connector=source-braintree

🕑 source-braintree https://github.com/airbytehq/airbyte/actions/runs/1123026558
❌ source-braintree https://github.com/airbytehq/airbyte/actions/runs/1123026558

avida · 2021-08-12T08:21:45Z

/test connector=source-braintree

🕑 source-braintree https://github.com/airbytehq/airbyte/actions/runs/1123158004
✅ source-braintree https://github.com/airbytehq/airbyte/actions/runs/1123158004

avida · 2021-08-16T13:17:45Z

/test connector=source-braintree

🕑 source-braintree https://github.com/airbytehq/airbyte/actions/runs/1135600523
✅ source-braintree https://github.com/airbytehq/airbyte/actions/runs/1135600523

Zirochkaa

few comments

airbyte-integrations/connectors/source-braintree/integration_tests/__init__.py

airbyte-integrations/connectors/source-braintree/setup.py

airbyte-integrations/builds.md

docs/integrations/sources/braintree-standalone.md

airbyte-integrations/connectors/source-braintree/unit_tests/__init__.py

airbyte-integrations/connectors/source-braintree/unit_tests/test_source.py

airbyte-integrations/connectors/source-braintree/acceptance-test-config.yml

airbyte-integrations/connectors/source-braintree/source_braintree/streams.py

avida · 2021-08-17T11:23:49Z

/test connector=source-braintree

🕑 source-braintree https://github.com/airbytehq/airbyte/actions/runs/1139056884
✅ source-braintree https://github.com/airbytehq/airbyte/actions/runs/1139056884

docs/integrations/sources/braintree-standalone.md

airbyte-integrations/connectors/source-braintree/source_braintree/streams.py

yevhenii-ldv · 2021-08-17T12:28:15Z

...c/main/resources/config/STANDARD_SOURCE_DEFINITION/63cea06f-1c75-458d-88fe-ad48c7cb27fd.json

@@ -0,0 +1,7 @@
+{
+  "sourceDefinitionId": "63cea06f-1c75-458d-88fe-ad48c7cb27fd",
+  "name": "Braintree",


Could you add "Deprecated" to the old one (for json and yml files)

yevhenii-ldv · 2021-08-17T12:38:24Z

airbyte-integrations/connectors/source-braintree/integration_tests/configured_catalog.json

+      "stream": {
+        "name": "customer_stream",
+        "json_schema": {},
+        "supported_sync_modes": ["full_refresh"],


I look at the code and don't understand why your customer_stream stream can be Incremental, but in the catalog you indicate it simply as full_refresh?

Isn't it ignored by acceptance test anyway? What point of those changes to integration_tests/configured_catalog.json ?

yevhenii-ldv · 2021-08-17T12:38:53Z

airbyte-integrations/connectors/source-braintree/integration_tests/configured_catalog.json

+        "json_schema": {},
+        "supported_sync_modes": ["full_refresh"],
+        "source_defined_cursor": false,
+        "default_cursor_field": ["column_name"]


And I don't see the source_defined_primary_key field

yevhenii-ldv · 2021-08-17T12:39:49Z

airbyte-integrations/connectors/source-braintree/integration_tests/configured_catalog.json

+        "source_defined_cursor": false,
+        "default_cursor_field": ["column_name"]


That is unnecessary

yevhenii-ldv · 2021-08-17T13:04:04Z

airbyte-integrations/connectors/source-braintree/source_braintree/streams.py

+            item = self.get_json_from_resource(item)
+            item = self.model(**item)
+            yield item.dict(exclude_unset=True)
+        yield from []


Why do you need this line - the code should complete without it?

yevhenii-ldv · 2021-08-17T13:20:12Z

airbyte-integrations/connectors/source-braintree/source_braintree/streams.py

+    @backoff.on_exception(
+        backoff.expo,
+        (
+            braintree.exceptions.GatewayTimeoutError,
+            braintree.exceptions.RequestTimeoutError,
+            braintree.exceptions.ServerError,
+            braintree.exceptions.ServiceUnavailableError,
+            braintree.exceptions.TooManyRequestsError,
+        ),
+        max_tries=5,
+    )
+    def read_records(


I'm not sure if this is correct.
I think that we should not wrap the whole generator in backoff, but a separate function that requests data once.
In this case, we exclude the likelihood that if an error pops up at the 10th iteration, then we will not run the read_records function with the initial values, thereby getting duplicate records.

Good point, fixed

yevhenii-ldv · 2021-08-17T13:24:59Z

docs/SUMMARY.md

    * [Braintree](integrations/sources/braintree.md)
    * [Bing Ads](integrations/sources/bing-ads.md)
    * [BigQuery](integrations/sources/bigquery.md)
+    * [Braintree (standalone)](integrations/sources/braintree-standalone.md)


Suggested change

* [Braintree](integrations/sources/braintree.md)

* [Bing Ads](integrations/sources/bing-ads.md)

* [BigQuery](integrations/sources/bigquery.md)

* [Braintree (standalone)](integrations/sources/braintree-standalone.md)

* [Braintree (Deprecated)](integrations/sources/braintree.md)

* [Bing Ads](integrations/sources/bing-ads.md)

* [BigQuery](integrations/sources/bigquery.md)

* [Braintree](integrations/sources/braintree-standalone.md)

yevhenii-ldv

Some comments

avida · 2021-08-18T07:46:28Z

/test connector=source-braintree

🕑 source-braintree https://github.com/airbytehq/airbyte/actions/runs/1142273098
✅ source-braintree https://github.com/airbytehq/airbyte/actions/runs/1142273098

yevhenii-ldv

LGTM

sherifnada

Nicely done with using pydantic for the models. It's super easy to read. Was it easier to use than JSON/would you prefer that we do things this way going forward?

We should remove the previous singer connector from the YAMLs, jsons, etc... (not just rename it). We should make things as simple as possible for the user, who probably doesn't care if something is based on singer or airbyte, they just want data from braintree

sherifnada · 2021-08-18T15:57:38Z

airbyte-config/init/src/main/resources/seed/source_definitions.yaml

@@ -168,7 +168,7 @@
  documentationUrl: https://docs.airbyte.io/integrations/sources/freshdesk
  icon: freshdesk.svg
 - sourceDefinitionId: 396e4ca3-8a97-4b85-aa4e-c9d8c2d5f992
-  name: Braintree
+  name: Braintree (Deprecated)


we should delete the singer one

sherifnada · 2021-08-18T16:16:09Z

airbyte-integrations/connectors/source-braintree/acceptance-test-config.yml

+      configured_catalog_path: "integration_tests/configured_catalog.json"
+      empty_streams: ["subscription_stream"]
+      expect_records:
+        path: "integration_tests/expected_records.txt"


great job adding expected records!

sherifnada · 2021-08-18T16:25:01Z

airbyte-integrations/connectors/source-braintree/source_braintree/schemas/common.py

+
+
+class AllOptional(pydantic.main.ModelMetaclass):
+    def __new__(self, name, bases, namespaces, **kwargs):


can you add a docstring to explain why we need this?

sherifnada · 2021-08-18T16:28:16Z

airbyte-integrations/connectors/source-braintree/source_braintree/schemas/customer.py

+    VenmoAccount,
+    VisaCheckoutCard,
+)
+from .common import CatalogModel as BaseModel


I think renaming it as basemodel is a little misleading because we're not using basemodel, we're using a subclass of it :)

sherifnada · 2021-08-18T16:29:01Z

airbyte-integrations/connectors/source-braintree/source_braintree/spec.py

+        name="Merchant ID",
+        description='<a href="https://docs.airbyte.io/integrations/sources/braintree">Merchant ID</a> is the unique identifier for entire gateway account.',
+    )
+    public_key: str = Field(name="Public key", description="This is your user-specific public identifier for Braintree.")


can we add the title field to each of these?

It would automatically be added by pydantic model generator:

{ "type": "SPEC", "spec": { "documentationUrl": "https://docs.airbyte.io/integrations/sources/braintree", "connectionSpecification": { "title": "Braintree Spec", "type": "object", "properties": { "merchant_id": { "title": "Merchant Id", "description": "<a href=\"https://docs.airbyte.io/integrations/sources/braintree\">Merchant ID</a> is the unique identifier for entire gateway account.", "name": "Merchant ID", "type": "string" }, "public_key": { "title": "Public Key", "description": "This is your user-specific public identifier for Braintree.", "name": "Public key", "type": "string" }, "private_key": { "title": "Private Key", "description": "This is your user-specific private identifier.", "name": "Private Key", "airbyte_secret": true, "type": "string" }, "start_date": { "title": "Start Date", "description": "The date from which you'd like to replicate data for Braintree API for UTC timezone, All data generated after this date will be replicated.", "name": "Start date", "examples": [ "2020", "2020-12-30", "2020-11-22 20:20:05" ], "type": "string", "format": "date-time" }, "environment": { "$ref": "#/definitions/Environment" } }, "required": [ "merchant_id", "public_key", "private_key", "environment" ], "definitions": { "Environment": { "title": "Environment", "description": "An enumeration.", "enum": [ "Development", "Sandbox", "Qa", "Production" ], "type": "string" } } } } }

sherifnada · 2021-08-18T16:32:55Z

airbyte-integrations/connectors/source-braintree/unit_tests/test_streams.py

+
+
+@responses.activate
+def test_customers_stream(test_config):


awesome use of this library. Nicely done!

sherifnada · 2021-08-18T16:33:48Z

airbyte-integrations/connectors/source-braintree/source_braintree/streams.py

+from source_braintree.spec import BraintreeConfig
+
+
+class BraintreeStream(Stream):


nit: we should declare as ABC since it declares abstract methods

Suggested change

class BraintreeStream(Stream):

class BraintreeStream(Stream, ABC):

avida · 2021-08-19T07:48:34Z

/test connector=source-braintree

🕑 source-braintree https://github.com/airbytehq/airbyte/actions/runs/1146114860
✅ source-braintree https://github.com/airbytehq/airbyte/actions/runs/1146114860

avida · 2021-08-19T07:51:31Z

Nicely done with using pydantic for the models. It's super easy to read. Was it easier to use than JSON/would you prefer that we do things this way going forward?

Yes, it was a lot more easy than JSON, I used pydantic models for Amazon Ads and Braintree and was happy about it, would like to use it instead of jsonschema models.

avida · 2021-08-19T07:56:27Z

/publish connector=source-braintree

❌ source-braintree https://github.com/airbytehq/airbyte/actions/runs/1146133843

avida · 2021-08-19T08:02:11Z

/publish connector=source-braintree

❌ source-braintree https://github.com/airbytehq/airbyte/actions/runs/1146149498

avida · 2021-08-19T08:38:22Z

/publish connector=connectors/source-braintree

🕑 connectors/source-braintree https://github.com/airbytehq/airbyte/actions/runs/1146258683
✅ connectors/source-braintree https://github.com/airbytehq/airbyte/actions/runs/1146258683

github-actions bot added the area/connectors Connector related issues label Aug 12, 2021

jrhizor temporarily deployed to more-secrets August 12, 2021 07:36 Inactive

avida force-pushed the drezchykov/5236-braintree-source branch from 2a43333 to fe956f1 Compare August 12, 2021 08:21

jrhizor temporarily deployed to more-secrets August 12, 2021 08:24 Inactive

avida requested review from htrueman, yevhenii-ldv and Zirochkaa August 12, 2021 15:54

avida force-pushed the drezchykov/5236-braintree-source branch from fe956f1 to 11508d7 Compare August 13, 2021 05:57

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

avida force-pushed the drezchykov/5236-braintree-source branch 5 times, most recently from ed6132d to f277359 Compare August 16, 2021 13:17

jrhizor temporarily deployed to more-secrets August 16, 2021 13:20 Inactive

Zirochkaa suggested changes Aug 17, 2021

View reviewed changes

avida requested a review from Zirochkaa August 17, 2021 11:22

jrhizor temporarily deployed to more-secrets August 17, 2021 11:26 Inactive

Zirochkaa suggested changes Aug 17, 2021

View reviewed changes

docs/integrations/sources/braintree-standalone.md Outdated Show resolved Hide resolved

airbyte-integrations/connectors/source-braintree/source_braintree/streams.py Outdated Show resolved Hide resolved

avida force-pushed the drezchykov/5236-braintree-source branch from b9dd023 to 3e8f0b7 Compare August 17, 2021 12:01

Zirochkaa approved these changes Aug 17, 2021

View reviewed changes

avida requested a review from sherifnada August 17, 2021 12:03

yevhenii-ldv reviewed Aug 17, 2021

View reviewed changes

yevhenii-ldv suggested changes Aug 17, 2021

View reviewed changes

avida requested a review from yevhenii-ldv August 18, 2021 07:46

jrhizor temporarily deployed to more-secrets August 18, 2021 07:49 Inactive

yevhenii-ldv approved these changes Aug 18, 2021

View reviewed changes

sherifnada approved these changes Aug 18, 2021

View reviewed changes

jrhizor temporarily deployed to more-secrets August 19, 2021 07:51 Inactive

avida force-pushed the drezchykov/5236-braintree-source branch from c55ce76 to 2fc1619 Compare August 19, 2021 07:58

Dmytro Rezchykov added 6 commits August 19, 2021 10:59

5236: Add Braintree source connector

e1cd487

Braintree: Register docker image and add docs

1cbb963

Fix review comments

a446588

Fix review comment

6a97c50

Fix review comments

9aed148

Fix review comments

5e4f747

avida force-pushed the drezchykov/5236-braintree-source branch from 2fc1619 to 5e4f747 Compare August 19, 2021 08:00

jrhizor temporarily deployed to more-secrets August 19, 2021 08:04 Inactive

jrhizor temporarily deployed to more-secrets August 19, 2021 08:40 Inactive

avida merged commit 5560df7 into master Aug 19, 2021

avida deleted the drezchykov/5236-braintree-source branch August 19, 2021 08:49

avida changed the title ~~Add Braintree source connector~~ 🎉 Add Braintree source connector Sep 1, 2021

igrankova added connectors/sources-api connectors/source/braintree labels Jan 5, 2022

		"source_defined_cursor": false,
		"default_cursor_field": ["column_name"]



		class AllOptional(pydantic.main.ModelMetaclass):
		def __new__(self, name, bases, namespaces, **kwargs):

		from source_braintree.spec import BraintreeConfig


		class BraintreeStream(Stream):

	class BraintreeStream(Stream):
	class BraintreeStream(Stream, ABC):

🎉 Add Braintree source connector #5362

🎉 Add Braintree source connector #5362

Conversation

avida commented Aug 12, 2021

What

How

Recommended reading order

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

avida commented Aug 12, 2021 • edited by github-actions bot Loading

avida commented Aug 12, 2021 • edited by github-actions bot Loading

avida commented Aug 16, 2021 • edited by github-actions bot Loading

Zirochkaa left a comment

Choose a reason for hiding this comment

avida commented Aug 17, 2021 • edited by github-actions bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yevhenii-ldv left a comment

Choose a reason for hiding this comment

avida commented Aug 18, 2021 • edited by github-actions bot Loading

yevhenii-ldv left a comment

Choose a reason for hiding this comment

sherifnada left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avida commented Aug 19, 2021 • edited by github-actions bot Loading

avida commented Aug 19, 2021

avida commented Aug 19, 2021 • edited by github-actions bot Loading

avida commented Aug 19, 2021 • edited by github-actions bot Loading

avida commented Aug 19, 2021 • edited by github-actions bot Loading

avida commented Aug 12, 2021 •

edited by github-actions bot

Loading

avida commented Aug 12, 2021 •

edited by github-actions bot

Loading

avida commented Aug 16, 2021 •

edited by github-actions bot

Loading

avida commented Aug 17, 2021 •

edited by github-actions bot

Loading

avida commented Aug 18, 2021 •

edited by github-actions bot

Loading

avida commented Aug 19, 2021 •

edited by github-actions bot

Loading

avida commented Aug 19, 2021 •

edited by github-actions bot

Loading

avida commented Aug 19, 2021 •

edited by github-actions bot

Loading

avida commented Aug 19, 2021 •

edited by github-actions bot

Loading