-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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: PersistIq #9515
🎉 New Source: PersistIq #9515
Access has been restricted
You have triggered a rate limit.
Please wait a few minutes before you try again;
in some cases this may take up to an hour.
Conversation
Hi @Zaimwa9, thank you for your contribution! |
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.
@Zaimwa9 I tried to run the acceptance tests but they are failing, please make sure they run successfully and I'll go for a second review.
Hey @alafanechere, Indeed, I was looking at the unit tests. I'm working on making them pass but I'm stuck on the read one. Would love a bit of help to understand how to pass it :) |
… into zaimwa9/persistiq-source
@Zaimwa9 I made some required changes on my side branch to make the acceptance pass. Could you please grant us permission to push on your fork (guide here) so that I can push my commits on it? |
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.
I made a first quick review of all the boilerplate files and detected some required changes.
Please check my comments.
I'll go for a python code review soon.
airbyte-integrations/connectors/source-persistiq/integration_tests/abnormal_state.json
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-persistiq/source_persistiq/bootstrap.md
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-persistiq/integration_tests/sample_state.json
Outdated
Show resolved
Hide resolved
...yte-integrations/connectors/source-persistiq/integration_tests/expected_campaigns_stream.txt
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-persistiq/source_persistiq/schemas/campaigns.json
Show resolved
Hide resolved
Requested changes done, thanks :) |
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.
Minor comment and suggestions. Let me know what you think.
airbyte-integrations/connectors/source-persistiq/source_persistiq/source.py
Show resolved
Hide resolved
airbyte-integrations/connectors/source-persistiq/source_persistiq/source.py
Show resolved
Hide resolved
airbyte-integrations/connectors/source-persistiq/source_persistiq/source.py
Show resolved
Hide resolved
airbyte-integrations/connectors/source-persistiq/source_persistiq/source.py
Show resolved
Hide resolved
@@ -0,0 +1,19 @@ | |||
{ | |||
"documentationUrl": "https://docsurl.com", |
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.
"documentationUrl": "https://docsurl.com", | |
"documentationUrl": "https://docs.airbyte.io/integrations/sources/persistiq", |
airbyte-integrations/connectors/source-persistiq/unit_tests/test_streams.py
Show resolved
Hide resolved
Could you let me know when you give me maintainer permission to push my commits on your branch? |
Thank you @Zaimwa9 for the changes. I'm running the test on our CI now and will let you know how it goes. |
@Zaimwa9 thanks for your contrib, I made minor changes to make the test pass. |
Thanks a lot, couldn't look at the evolution earlier. 🎉 |
What
This PR adds a new source: PersistIq api
Comments
Not sure about badge.md
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/SUMMARY.md
docs/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 changesThis change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)