🐛 Source Pardot: failed while syncing Visitors stream #19618

MJafarMashhadi · 2022-11-18T21:04:03Z

What

Pardot connector failed during syncing Visitors stream due to an interface change. The method stream_slices of Stream class (in CDK) was refactored to only accept keyword arguments (#9037) and that change was not reflected in source-pardot connector.

How

This small PR updates the aforementioned method's call signature.

🚨 User Impact 🚨

No breaking changes.

Pre-merge Checklist

Not sure if I need to apply manually or there is some automation:
[ ] Add a row in changelog in pardot.md

Updated 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
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

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
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 and connector version bumped 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

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

CLAassistant · 2022-11-18T21:04:08Z

All committers have signed the CLA.

marcosmarxm · 2022-11-22T20:26:09Z

Hello 👋, first thank you for this amazing contribution.

We really appreciate the effort you've made to improve the project.
We ask you patience for the code review. Last month our team was focused on Hacktoberfest event and that probably left some PR without the proper feedback. And this week, due to the Thanksgiving US Holiday, most our team is out of office with their families. Another important piece of information why code won't be merge this week is: as a safety measure the core team has decided to freeze merging code to main branch to keep the release stable. Next week we'll return to you with the proper code review and update the status of your contribution.

If you have any questions feel free to send me a message in Slack!
Thanks!

haithem-souala · 2022-12-05T19:02:28Z

Hey @MJafarMashhadi, thank you for your contribution.
I will check your PR ASAP. @sajarin

haithem-souala · 2022-12-05T19:32:37Z

@MJafarMashhadi, could you please check theses suggestions:

spec.json

Could you modify the documentationUrl to https://docs.airbyte.com/integrations/sources/pardot

source.py

line 37, i would return Visitors(**args) directly as other streams, instead of visitors = Visitors(**args)

Could you also, share screenshot of the integration and acceptance tests.

sajarin · 2022-12-12T18:26:25Z

@MJafarMashhadi can you provide screenshots of passing review or acceptance tests please so we can get this merged? Thanks!

sajarin · 2022-12-14T19:41:36Z

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

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

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

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

sajarin · 2022-12-15T22:48:30Z

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

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

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

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

sajarin · 2022-12-15T23:08:46Z

Thanks @MJafarMashhadi for the PR and @haithem-souala for the review!

evantahler · 2022-12-19T19:36:13Z

FYI this Pull Request did not update the source-pardot README's changelog. It will be fixed up via #20666.

MJafarMashhadi · 2023-01-20T21:05:53Z

Sorry I missed your comments since I was on vacation! I'm glad that this is fixed and merged.
We're using this patch in production so rest assured that it works

octavia-squidington-iv added area/connectors Connector related issues bounty community connectors/source/pardot labels Nov 18, 2022

MJafarMashhadi added 2 commits November 18, 2022 14:27

Update the kw-only argument

ef6adec

Update pardot version

a2b849e

MJafarMashhadi force-pushed the pardot-fix branch from 59a3bf0 to a2b849e Compare November 18, 2022 21:27

MJafarMashhadi added 2 commits November 21, 2022 08:00

Merge branch 'master' into pardot-fix

608daa6

Merge branch 'master' into pardot-fix

5b9639d

Merge branch 'master' into pardot-fix

8ceb86c

sajarin added the bounty-S Maintainer program: claimable small bounty PR label Dec 5, 2022

sajarin assigned haithem-souala Dec 5, 2022

MJafarMashhadi and others added 5 commits December 6, 2022 11:25

Merge branch 'master' into pardot-fix

3bd1bd1

Merge branch 'master' into pardot-fix

7eb4091

Merge branch 'master' into pardot-fix

3574cc6

fix: address review changes

52449a1

Merge branch 'master' into pardot-fix

1ab66ff

sajarin added 2 commits December 14, 2022 13:37

Merge branch 'master' into pardot-fix

7e83fd5

Merge branch 'master' into pardot-fix

68b3c7e

sajarin approved these changes Dec 14, 2022

View reviewed changes

fix: update source.py to pass flakeCheck

a72a7e0

sajarin added 2 commits December 15, 2022 12:19

fix: delete incomplete test

43f7f7c

Merge branch 'master' into pardot-fix

a85d45f

auto-bump connector version

cb5b0a4

sajarin merged commit 32015af into airbytehq:master Dec 15, 2022

octavia-squidington-iii mentioned this pull request Dec 18, 2022

Bump Airbyte version from 0.40.25 to 0.40.26 #20630

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 Source Pardot: failed while syncing Visitors stream #19618

🐛 Source Pardot: failed while syncing Visitors stream #19618

MJafarMashhadi commented Nov 18, 2022

CLAassistant commented Nov 18, 2022 •

edited

Loading

marcosmarxm commented Nov 22, 2022

haithem-souala commented Dec 5, 2022 •

edited

Loading

haithem-souala commented Dec 5, 2022 •

edited

Loading

sajarin commented Dec 12, 2022

sajarin commented Dec 14, 2022 •

edited by github-actions bot

Loading

sajarin commented Dec 15, 2022 •

edited by github-actions bot

Loading

sajarin commented Dec 15, 2022 •

edited

Loading

evantahler commented Dec 19, 2022 •

edited

Loading

MJafarMashhadi commented Jan 20, 2023 •

edited

Loading

🐛 Source Pardot: failed while syncing Visitors stream #19618

🐛 Source Pardot: failed while syncing Visitors stream #19618

Conversation

MJafarMashhadi commented Nov 18, 2022

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

CLAassistant commented Nov 18, 2022 • edited Loading

marcosmarxm commented Nov 22, 2022

haithem-souala commented Dec 5, 2022 • edited Loading

haithem-souala commented Dec 5, 2022 • edited Loading

sajarin commented Dec 12, 2022

sajarin commented Dec 14, 2022 • edited by github-actions bot Loading

sajarin commented Dec 15, 2022 • edited by github-actions bot Loading

sajarin commented Dec 15, 2022 • edited Loading

evantahler commented Dec 19, 2022 • edited Loading

MJafarMashhadi commented Jan 20, 2023 • edited Loading

CLAassistant commented Nov 18, 2022 •

edited

Loading

haithem-souala commented Dec 5, 2022 •

edited

Loading

haithem-souala commented Dec 5, 2022 •

edited

Loading

sajarin commented Dec 14, 2022 •

edited by github-actions bot

Loading

sajarin commented Dec 15, 2022 •

edited by github-actions bot

Loading

sajarin commented Dec 15, 2022 •

edited

Loading

evantahler commented Dec 19, 2022 •

edited

Loading

MJafarMashhadi commented Jan 20, 2023 •

edited

Loading