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.

Sign up for GitHub

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

Jump to bottom

🎉 Source Zendesk: Migration from Singer to CDK #4861

Merged
merged 174 commits into from
Aug 2, 2021
Merged

🎉 Source Zendesk: Migration from Singer to CDK #4861

merged 174 commits into from
Aug 2, 2021

Conversation

antixar
Copy link
Contributor

@antixar antixar commented Jul 20, 2021 •
edited
Loading

What

Resolving the issue:
#4422 - New source: airbyte-native Zendesk Support

How

  • Using Airbyte Python CDK implemented the logic for the source connector, covering features present in singer-based Source Zendesk

Pre-merge Checklist

  • Issue acceptance criteria met
  • PR name follows PR naming conventions
  • Secrets are annotated with airbyte_secret in the connector's spec
  • Credentials added to Github CI if needed and not already present. instructions for injecting secrets into CI.
  • Unit & integration tests added as appropriate (and are passing)
    ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • /test connector=connectors/<name> command as documented here is passing.
  • Code reviews completed
  • Documentation updated
    • README.md
    • docs/SUMMARY.md if it's a new connector
    • Created or updated reference docs in docs/integrations/<source or destination>/<name>.
    • Changelog in the appropriate page in docs/integrations/<source or destination>/<name>. See changelog example
    • docs/integrations/README.md contains a reference to the new connector
    • Build status added to build page
  • Connector version bumped like described here
  • New Connector version released on Dockerhub by running the /publish command described here
  • Build is successful
  • No major blockers
  • PR merged into master branch
  • Follow up tickets have been created
  • Associated tickets have been closed & stakeholders notified

@github-actions github-actions bot added the area/connectors Connector related issues label Jul 20, 2021
@antixar antixar linked an issue Jul 20, 2021 that may be closed by this pull request
New source: airbyte-native Zendesk Support #4422
Closed
@antixar antixar self-assigned this Jul 20, 2021
midavadim
midavadim previously requested changes Jul 20, 2021
View reviewed changes
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/spec.json Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
@midavadim
Copy link
Contributor

midavadim commented Jul 20, 2021 •
edited by github-actions bot
Loading

/test connector=connectors/source-zendesk-support

🕑 connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1050596658
❌ connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1050596658

@midavadim
Copy link
Contributor

In docs pages - please specify which streams support incremental sync

@bazarnov bazarnov changed the title Source Zendesk: Migration from Single to CDK 🎉 Source Zendesk: Migration from Single to CDK Jul 21, 2021
bazarnov
bazarnov previously requested changes Jul 21, 2021
View reviewed changes
Copy link
Collaborator

@bazarnov bazarnov left a comment

Choose a reason for hiding this comment

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

Why do you need the source-jira/integration_tests/configured_catalog.json ?

I assume it's added to this branch accidentally. Please correct this before you merge.

@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Jul 21, 2021
bazarnov
bazarnov reviewed Jul 21, 2021
View reviewed changes
Copy link
Collaborator

@bazarnov bazarnov left a comment

Choose a reason for hiding this comment

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

Remove this file from your branch

airbyte-integrations/connectors/source-us-census/integration_tests/integration_test.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-jira/source_jira/schemas/labels.json Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-jira/sample_files/full_configured_catalog.json Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-jira/sample_files/configured_catalog.json Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-jira/integration_tests/labels_catalog.json Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
@antixar antixar requested review from midavadim and bazarnov July 22, 2021 12:11
sherifnada and others added 2 commits July 22, 2021 15:32
@sherifnada @gl-pix
🎉 Python CDK: Allow setting network adapter args on outgoing HTTP req…
f02d74a 
…uests (#4493)
@tuliren @gl-pix
🎉 Destination S3: support anyOf allOf and oneOf (#4613)
a9ae0eb 
* Support combined restrictions in json schema

* Bump s3 version

* Add more test cases

* Update changelog

* Add more test cases

* Update documentation

* Format code
@antixar antixar requested a review from sherifnada July 29, 2021 00:48
@antixar
Copy link
Contributor Author

antixar commented Jul 29, 2021 •
edited by github-actions bot
Loading

/test connector=connectors/source-zendesk-support

🕑 connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1077630748
❌ connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1077630748

@antixar
Copy link
Contributor Author

antixar commented Jul 29, 2021

@sherifnada , you are right that the Zendesk API design is not uniform. It provides different mechanisms of filters and pagination for similar entities. For example the query parameter of the future response size can have the following keys: per_page, count, limit. Thus we forced to implement a lot of customization.
I made a refactoring considering your comments.

@antixar
Copy link
Contributor Author

antixar commented Jul 29, 2021 •
edited by github-actions bot
Loading

/test connector=connectors/source-zendesk-support

🕑 connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1077962634
✅ connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1077962634

sherifnada
sherifnada reviewed Jul 30, 2021
View reviewed changes
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

Apologies for the delayed review, had some more comments. Can you also add some unit tests verifying the functionality of the different classes? the requests-mock library may be helpful in achieving this

airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/source.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-zendesk-support/source_zendesk_support/streams.py Outdated Show resolved Hide resolved
@antixar antixar requested a review from sherifnada July 30, 2021 17:35
@antixar
Copy link
Contributor Author

antixar commented Jul 30, 2021 •
edited by github-actions bot
Loading

/test connector=connectors/source-zendesk-support

🕑 connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1083064087
❌ connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1083064087

@antixar
Copy link
Contributor Author

antixar commented Jul 30, 2021

@sherifnada , thank you for your constructive comments. Please check my last updates.
I've added a set of tests for more fragile logic parts

@antixar
Copy link
Contributor Author

antixar commented Aug 2, 2021 •
edited by github-actions bot
Loading

/test connector=connectors/source-zendesk-support

🕑 connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1090134500
❌ connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1090134500

@jrhizor jrhizor temporarily deployed to more-secrets August 2, 2021 12:06 Inactive
@antixar
Copy link
Contributor Author

antixar commented Aug 2, 2021 •
edited by github-actions bot
Loading

/test connector=connectors/source-zendesk-support

🕑 connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1090217231
❌ connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1090217231

@antixar antixar closed this Aug 2, 2021
@antixar
Copy link
Contributor Author

antixar commented Aug 2, 2021 •
edited by github-actions bot
Loading

/test connector=connectors/source-zendesk-support

🕑 connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1090217995
❌ connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1090217995

@jrhizor jrhizor temporarily deployed to more-secrets August 2, 2021 12:33 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets August 2, 2021 12:33 Inactive
@antixar
Copy link
Contributor Author

antixar commented Aug 2, 2021 •
edited by github-actions bot
Loading

/test connector=connectors/source-zendesk-support

🕑 connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1090390790
✅ connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1090390790

@antixar antixar reopened this Aug 2, 2021
@antixar
Copy link
Contributor Author

antixar commented Aug 2, 2021 •
edited by github-actions bot
Loading

/publish connector=connectors/source-zendesk-support

🕑 connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1090454917
❌ connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1090454917

@jrhizor jrhizor temporarily deployed to more-secrets August 2, 2021 13:48 Inactive
@antixar
Copy link
Contributor Author

antixar commented Aug 2, 2021 •
edited by github-actions bot
Loading

/publish connector=connectors/source-zendesk-support

🕑 connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1090482694
✅ connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/1090482694

@jrhizor jrhizor temporarily deployed to more-secrets August 2, 2021 13:58 Inactive
@antixar antixar merged commit 85b0bb2 into master Aug 2, 2021
@antixar antixar deleted the antixar/4422-source-zendesk-support branch August 2, 2021 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@midavadim midavadim midavadim left review comments

@lazebnyi lazebnyi lazebnyi approved these changes

@TymoshokDmytro TymoshokDmytro TymoshokDmytro approved these changes

@sherifnada sherifnada sherifnada approved these changes

@tuliren tuliren tuliren approved these changes

@annalvova05 annalvova05 Awaiting requested review from annalvova05

@yaroslav-dudar yaroslav-dudar Awaiting requested review from yaroslav-dudar

@bazarnov bazarnov Awaiting requested review from bazarnov

Assignees

@antixar antixar

Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation
Projects
No open projects
GL Roadmap
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

New source: airbyte-native Zendesk Support Zendesk not loading username and facebook/twitter id