Skip to content
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

🎉 New Source: GoCardless #17792

Merged
merged 13 commits into from
Oct 19, 2022
Merged

Conversation

isaacharrisholt
Copy link
Contributor

@isaacharrisholt isaacharrisholt commented Oct 10, 2022

What

Airbyte supports retrieving data from payment providers such as Stripe, but not GoCardless, a payment provider for direct debit transactions. This is data that can be crucial for financial reconciliation and forecasting.

How

This PR uses the lowcode CDK to create a GoCardless connector with the following streams:

  • payments
  • refunds
  • mandates
  • payouts

The goal is to add more in the future.

Recommended reading order

  1. gocardless.yaml
  2. spec.yaml
  3. configured_catalog.json

🚨 User Impact 🚨

No changes to existing code.

Pre-merge Checklist

New Connector

Community member or Airbyter

  • Community member? 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
    • docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
    • docs/integrations/README.md
    • airbyte-integrations/builds.md
  • 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
  • If new credentials are required for use in CI, add them to GSM. Instructions.
  • /test connector=connectors/<name> command is passing
  • New Connector version released on Dockerhub by running the /publish command described here
  • After the connector is published, connector added to connector index 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

Tests

Unit

image

Integration

image

Acceptance

image

@CLAassistant
Copy link

CLAassistant commented Oct 10, 2022

CLA assistant check
All committers have signed the CLA.

@marcosmarxm
Copy link
Member

Please sign the CLA @isaacharrisholt

Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few comments, but look good!

Comment on lines 18 to 24
gocardless_environment:
title: Gocardless API Environment
type: string
pattern: "^(live|sandbox)$"
description: |
Environment you are trying to connect to. Should be either `live` or
`sandbox`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we transform this to enum with live and sandbox options?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Though I must say I couldn't find enum in the connector spec reference. Maybe I'm blind, but it would be nice if the docs had full-text search :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, there isn't anything there. The docs must be updated.

before this date will not be replicated.
examples:
- '2017-01-25T00:00:00Z'
lookback_window_days:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@girarda this parameter is used internally for the low-code cdk?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a relic from a previous version of this connector I had running. I just forgot to remove it. That has now been done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome

@isaacharrisholt
Copy link
Contributor Author

Please sign the CLA @isaacharrisholt

I signed it yesterday, but the CLA Assistant seems to disagree. Here's evidence:

Screenshot_20221011-182704

@sajarin sajarin added internal and removed bounty labels Oct 12, 2022
@marcosmarxm
Copy link
Member

@isaacharrisholt sometimes people submit the code using other account/email from the local configuration.

Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small details, overall lgtm!

@@ -0,0 +1,45 @@
documentationUrl: https://docsurl.com
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 7 to 10
- access_token
- gocardless_environment
- gocardless_version
- start_date
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 spaces 👮

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@marcosmarxm
Copy link
Member

after changing the 2 last comments I think this contribution is ready to merge!

@marcosmarxm marcosmarxm self-assigned this Oct 17, 2022
@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Oct 19, 2022
@marcosmarxm
Copy link
Member

marcosmarxm commented Oct 19, 2022

/publish connector=connectors/source-gocardless run-tests=false

🕑 Publishing the following connectors:
connectors/source-gocardless
https://github.com/airbytehq/airbyte/actions/runs/3285375696


Connector Did it publish? Were definitions generated?
connectors/source-gocardless

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@marcosmarxm marcosmarxm merged commit 25feff7 into airbytehq:master Oct 19, 2022
@marcosmarxm
Copy link
Member

@RealChrisSean another one! :)

@RealChrisSean
Copy link

Noted!

jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
* Initial commit

* Reformat JSON

* Remove block from acceptance test yaml

* Remove references to lookback_window_days

* Make gocardless_environment an enum

* Add ordering to spec

* Address final comments

* Make commit to change email associated with PR

* Revert previous

* add seed config

* add docs

* auto-bump connector version

Co-authored-by: Marcos Marx <marcosmarxm@users.noreply.github.com>
Co-authored-by: marcosmarxm <marcosmarxm@gmail.com>
Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants