-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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: Partnerstack [low-code CDK] #18603
🎉 New source: Partnerstack [low-code CDK] #18603
Conversation
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.
A few comments but the connector looks solid. Please move request my review again when this is ready :)
airbyte-integrations/connectors/source-partnerstack/source_partnerstack/spec.yaml
Show resolved
Hide resolved
airbyte-integrations/connectors/source-partnerstack/source_partnerstack/spec.yaml
Outdated
Show resolved
Hide resolved
type: DefaultPaginator | ||
pagination_strategy: | ||
type: "CursorPagination" | ||
cursor_value: "{{ last_records[-1]['key'] }}" |
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.
Just curious about the usage of this option.
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.
Which option are you referring to?
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.
the cursor field, this is the first connector using this feature interpolation
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.
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.
@elliottrabac I saw this connector supports incremental syncs. Are you interested in implementing this sync mode for this connector? I'm askng because the incremental sync is much more interesting to users where you can retrieve only new records.
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.
Yes sure! Curious to see how it goes with the low code CDK 👍
I'll ping you once done.
@elliottrabac do you have a test account to share? |
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.
@elliottrabac can you share the output of ./gradlew airbyte-integrations:connectors:source-partnerstack:integrationTest
?
Unfortunately no :/ I tried all the streams locally and on the VM, working well: |
type: DefaultPaginator | ||
pagination_strategy: | ||
type: "CursorPagination" | ||
cursor_value: "{{ last_records[-1]['key'] }}" |
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.
@elliottrabac I saw this connector supports incremental syncs. Are you interested in implementing this sync mode for this connector? I'm askng because the incremental sync is much more interesting to users where you can retrieve only new records.
@elliottrabac please request my review again when you finish the to solve the comments. |
Hello! I'm going to be out of the office this Friday and won't be able to review your contribution again today, I return next Monday. So far, most contributions look solid and are almost done to be approved. As said in Chris' comment all contributions made before 2-November are eligible to receive the prize and have 2 weeks to merge the contributions. But I ensure next week we're going to have your contribution merged. If you have questions about the implementation you can send them in Sorry the inconvenience and see you again next week, thank you so much for your contribution! |
Hi @marcosmarxm I tried to implement the incremental sync for this connector but I'm struggling with datetimeformat with the low-code CDK because the format in Partnerstack is epoch timestamp (in ms).
I'm not sure I'll have time to wrap it up in the coming days. As the full refresh connector is working well, do you think we could merge it like that to meet the hacktoberfest deadline and iterate on the incremental sync after? |
@marcosmarxm I implemented a custom stream slice similar to the GreenHouse one. |
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.
Thanks @elliottrabac I'll publish the connector during the week.
/publish connector=connectors/source-partnerstack run-tests=false
if you have connectors that successfully published but failed definition generation, follow step 4 here |
* add boilerplate * configure schemas * add connector doc * update airbyte docs * edit spec * configure incremental sync * use custom components * add incremental acceptances tests * add partnerstack to source def * auto-bump connector version Co-authored-by: marcosmarxm <[email protected]> Co-authored-by: Octavia Squidington III <[email protected]>
What
Describe what the change is solving
It helps to add screenshots if it affects the frontend.
Hacktoberfest: airbytehq/connector-contest#207
How
Describe the solution
Recommended reading order
x.java
y.python
🚨 User Impact 🚨
Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.
Pre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampledocs/integrations/README.md
airbyte-integrations/builds.md
Airbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing/publish
command described hereUpdating a connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing/publish
command described hereConnector Generator
-scaffold
in their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates
then checking in your changesTests
Unit
Put your unit tests output here.
Integration
Put your integration tests output here.
Acceptance
Put your acceptance tests output here.