Source Plaid: port to Python CDK #7977

firmbase-tal · 2021-11-15T13:32:40Z

What

Reformatted plaid to be written in Python instead of Javascript: #3998
Added transaction functionality to the source.

How

The new Python connector works with the Plaid API package to draw data, then extracts the relevant data to stream, and dumps it as messages.
Balance stream maintains data structure from previous connector version.

Pre-merge Checklist

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
- Connector's bootstrap.md. See description and examples
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions

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
After the new connector version is published, connector version bumped in the seed directory as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

CLAassistant · 2021-11-15T13:32:47Z

All committers have signed the CLA.

alafanechere · 2021-11-18T09:35:42Z

Hi @firmbase-tal , thanks for this contribution! Could you please share evidence that acceptance tests are passing on your side?
Command to run from top level airbyte directory:

./gradlew :airbyte-integrations:connectors:source-plaid:integrationTest

Please also run:

./gradlew bash

alafanechere

I made a first quick review of the core and my main concern is that I feel like the abstractions you defined thanks to configs/PlaidStream/PlaidRequester are a bit far-fetched. I'm not 100% sure of the interest of storing some streams configurations in an external json file.
I think that you can make your connector more readable by having the following classes:

PlaidStream(Stream)
BalanceStream(PlaidStream)
IncrementalTransactionStream(PlaidStream)

And IMO the PlaidRequesters method could be brought back to the child classes above.

Let me know what you think about this!

alafanechere · 2021-11-18T09:22:25Z

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

+    - config_path: "secrets/config.json"
+      configured_catalog_path: "integration_tests/configured_catalog.json"
+      empty_streams: []
+  # TODO uncomment this block to specify that the tests should assert the connector outputs the records provided in the input file a file


Please remove all TODO

Hi @firmbase-tal , thanks for this contribution! Could you please share evidence that acceptance tests are passing on your side? Command to run from top level airbyte directory:

./gradlew :airbyte-integrations:connectors:source-plaid:integrationTest

Please also run:

./gradlew bash

./gradlew bash

This fails with error: Task 'bash' not found in root project 'airbyte'.
I'm unsure as to what went wrong there.

Also, where should I upload the files from the acceptance tests?

Sorry it was a typo for my side I meant ./gradlew format

airbyte-integrations/connectors/source-plaid/README.md

alafanechere · 2021-11-23T15:05:57Z

Hi @firmbase-tal, are you still interested in moving this PR forward according to my review?

alafanechere · 2021-12-01T10:15:40Z

I add the label blocked until the user reply the review.

Co-authored-by: Augustin <augustin@airbyte.io>

firmbase-tal · 2021-12-13T09:29:29Z

Yes, apologies for the delay, will handle the changes soon.

hbd · 2021-12-23T21:31:48Z

Thanks for your work here @firmbase-tal. Let me know if I can help get this over the line.

firmbase-tal · 2021-12-27T13:02:34Z

I think I fixed everything for now, please let me know if further changes are necessary.

alafanechere · 2021-12-27T18:23:30Z

Hi @firmbase-tal, thanks for the changes; I'll go for another review this week then! @hbd feel free to review too if you're familiar with Plaid 😄

alafanechere · 2022-01-05T18:27:20Z

@firmbase-tal I allowed myself to make some changes to simplify the code.
Mainly:

Schemas are usually declared in external json files.
The PlaidRequester class and some abstraction logic looked redundant to me.

Let me know what you think about these.

hbd

LGTM. I was able to build the image locally and, although I am not very familiar with the Plaid API, the spec looks good

airbyte-integrations/connectors/source-plaid/source_plaid/source.py

alafanechere

I confirm acceptance tests are passing. I'm publishing this now.

hbd · 2022-01-10T03:50:01Z

Just confirming that I ran latest locally and successfully synced Plaid Sandbox data.

alafanechere · 2022-01-10T11:16:00Z

Thank you @hbd for testing and @firmbase-tal for your original contribution!

firmbase-tal added 2 commits November 15, 2021 15:13

Switched plaid source to Python, and added transaction support.

4283f4e

Merge branch 'master' of https://github.com/firmbase-tal/airbyte

4405bda

github-actions bot added the area/connectors Connector related issues label Nov 15, 2021

octavia-squidington-iii added the community label Nov 15, 2021

firmbase-tal changed the title ~~Plaid source update~~ Migrate plaid source to Python CDK and add transaction functionality Nov 15, 2021

alafanechere self-assigned this Nov 15, 2021

alafanechere suggested changes Nov 18, 2021

View reviewed changes

alafanechere added the hacktober label Nov 23, 2021

alafanechere added the blocked label Dec 1, 2021

Update airbyte-integrations/connectors/source-plaid/README.md

5808847

Co-authored-by: Augustin <augustin@airbyte.io>

firmbase-tal added 5 commits December 27, 2021 13:19

Fixes to plaid for integration tests

d4ff4f2

Removed null entries from records

fef1d1f

Revereted filtering null, made schema allow null instead.

3638bf2

Added access_token to invalid config

caaf120

Removed catalog.json from code and split to multiple stream classes.

080352d

Fixed linter errors.

1289395

alafanechere removed the blocked label Dec 27, 2021

alafanechere mentioned this pull request Dec 30, 2021

Publish #7977 #9227

Closed

alafanechere temporarily deployed to more-secrets December 30, 2021 18:28 Inactive

jrhizor temporarily deployed to more-secrets December 30, 2021 18:29 Inactive

Fixed format errors.

4be0aaa

alafanechere temporarily deployed to more-secrets January 4, 2022 14:00 Inactive

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

alafanechere temporarily deployed to more-secrets January 5, 2022 18:25 Inactive

jrhizor temporarily deployed to more-secrets January 5, 2022 18:27 Inactive

remove plaid requester

05b3df2

alafanechere temporarily deployed to more-secrets January 5, 2022 18:30 Inactive

hbd reviewed Jan 5, 2022

View reviewed changes

airbyte-integrations/connectors/source-plaid/source_plaid/source.py Outdated Show resolved Hide resolved

alafanechere added 2 commits January 6, 2022 18:35

format

169687a

remove unused import

d4395c1

alafanechere changed the title ~~Migrate plaid source to Python CDK and add transaction functionality~~ Source Plaid: port to Python CDK Jan 6, 2022

alafanechere approved these changes Jan 6, 2022

View reviewed changes

Merge branch 'master' into augustin/publish-7977

cc9b22d

jrhizor temporarily deployed to more-secrets January 6, 2022 18:08 Inactive

jrhizor temporarily deployed to more-secrets January 6, 2022 18:18 Inactive

jrhizor temporarily deployed to more-secrets January 7, 2022 07:37 Inactive

update source specs

fe2d70d

alafanechere temporarily deployed to more-secrets January 7, 2022 08:15 Inactive

Merge branch 'master' into augustin/publish-7977

aa7ca0b

alafanechere temporarily deployed to more-secrets January 7, 2022 14:50 Inactive

Merge branch 'master' into augustin/publish-7977

9cf3e38

alafanechere temporarily deployed to more-secrets January 10, 2022 08:28 Inactive

alafanechere merged commit e68c564 into airbytehq:master Jan 10, 2022

alexells mentioned this pull request Jan 11, 2022

🎉 Source Plaid: enable transaction results paging #9402

Closed

16 tasks

octavia-squidington-iii mentioned this pull request Jan 11, 2022

Bump Airbyte version from 0.35.4-alpha to 0.35.5-alpha #9421

Merged

hbd mentioned this pull request Feb 10, 2022

port plaid source to Python CDK #3998

Closed

marcosmarxm removed the hacktober label Oct 20, 2022

octavia-squidington-iv added bounty connectors/source/plaid labels Oct 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Source Plaid: port to Python CDK #7977

Source Plaid: port to Python CDK #7977

firmbase-tal commented Nov 15, 2021 •

edited

Loading

CLAassistant commented Nov 15, 2021 •

edited

Loading

alafanechere commented Nov 18, 2021 •

edited

Loading

alafanechere left a comment

alafanechere Nov 18, 2021

firmbase-tal Dec 27, 2021

alafanechere Dec 30, 2021

alafanechere commented Nov 23, 2021

alafanechere commented Dec 1, 2021

firmbase-tal commented Dec 13, 2021

hbd commented Dec 23, 2021

firmbase-tal commented Dec 27, 2021

alafanechere commented Dec 27, 2021

alafanechere commented Jan 5, 2022

hbd left a comment

alafanechere left a comment

hbd commented Jan 10, 2022

alafanechere commented Jan 10, 2022

Source Plaid: port to Python CDK #7977

Source Plaid: port to Python CDK #7977

Conversation

firmbase-tal commented Nov 15, 2021 • edited Loading

What

How

Recommended reading order

Pre-merge Checklist

Community member or Airbyter

Airbyter

CLAassistant commented Nov 15, 2021 • edited Loading

alafanechere commented Nov 18, 2021 • edited Loading

alafanechere left a comment

Choose a reason for hiding this comment

alafanechere Nov 18, 2021

Choose a reason for hiding this comment

firmbase-tal Dec 27, 2021

Choose a reason for hiding this comment

alafanechere Dec 30, 2021

Choose a reason for hiding this comment

alafanechere commented Nov 23, 2021

alafanechere commented Dec 1, 2021

firmbase-tal commented Dec 13, 2021

hbd commented Dec 23, 2021

firmbase-tal commented Dec 27, 2021

alafanechere commented Dec 27, 2021

alafanechere commented Jan 5, 2022

hbd left a comment

Choose a reason for hiding this comment

alafanechere left a comment

Choose a reason for hiding this comment

hbd commented Jan 10, 2022

alafanechere commented Jan 10, 2022

firmbase-tal commented Nov 15, 2021 •

edited

Loading

CLAassistant commented Nov 15, 2021 •

edited

Loading

alafanechere commented Nov 18, 2021 •

edited

Loading