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: Zendesk Chat #3088

Merged
merged 7 commits into from
May 3, 2021
Merged

Conversation

yevhenii-ldv
Copy link
Contributor

What

Creating new source Zendesk Chat based on Base Python (FULL REFRESH)

closes #3039

How

Describe the solution

Pre-merge Checklist

  • Run integration tests
  • Publish Docker images

Recommended reading order

  1. test.java
  2. component.ts
  3. the rest

@auto-assign auto-assign bot requested review from cgardens and sherifnada April 27, 2021 17:14
@yevhenii-ldv yevhenii-ldv requested review from keu, vitaliizazmic, vovavovavovavova and Zirochkaa and removed request for sherifnada and cgardens April 27, 2021 17:15
@yevhenii-ldv
Copy link
Contributor Author

yevhenii-ldv commented Apr 27, 2021

/test connector=source-zendesk-chat

🕑 source-zendesk-chat https://github.com/airbytehq/airbyte/actions/runs/789950265
✅ source-zendesk-chat https://github.com/airbytehq/airbyte/actions/runs/789950265

Copy link
Contributor

@keu keu left a comment

Choose a reason for hiding this comment

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

you need to run format

…/new-connector-zendesk-chat

� Conflicts:
�	docs/integrations/connector-health.md
@yevhenii-ldv
Copy link
Contributor Author

you need to run format

Already formatted

from base_python import HttpStream


class ZendeskChatStream(HttpStream, ABC):
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't seem to have any abstract methods, why is it extending ABC?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it in PR for Support Incremental #3157

@@ -0,0 +1,16 @@
FROM airbyte/integration-base-python:0.1.4
Copy link
Contributor

Choose a reason for hiding this comment

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

update to latest

Suggested change
FROM airbyte/integration-base-python:0.1.4
FROM airbyte/integration-base-python:0.1.6

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated it in PR for Support Incremental #3157

stream_data = self.get_stream_data(response_data)

# Getting rid of duplicates
if self.pagination_support and response.url.find("since_id") > -1:
Copy link
Contributor

Choose a reason for hiding this comment

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

huh, very interesting problem here. I see why we're doing this. FWIW we can always emit duplicates because Airbyte is at-least-once-delivery. But this is OK 👍🏼

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it in the PR for Incremental, because in the case of the state, it could behave incorrectly, and hide the records

data_field = "chats"


class Shortcuts(ZendeskChatStream):
Copy link
Contributor

Choose a reason for hiding this comment

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

lol. Very concise code. Love it.

@keu keu self-requested a review April 30, 2021 21:19
eugene-kulak and others added 2 commits May 3, 2021 21:56
* Zendesk Chat: Implement Incremental sync

* Roll back the stream Chats to the previous endpoint

* update integration-base-python to v0.1.6

* update limit to 100

* add acceptence tests

* update test and setup configs

* remove unknown file

* update naming of classes

Co-authored-by: ykurochkin <y.kurochkin@zazmic.com>
Co-authored-by: Eugene Kulak <kulak.eugene@gmail.com>
@keu
Copy link
Contributor

keu commented May 3, 2021

/test connector=source-zendesk-chat

🕑 source-zendesk-chat https://github.com/airbytehq/airbyte/actions/runs/807800368
✅ source-zendesk-chat https://github.com/airbytehq/airbyte/actions/runs/807800368

@keu
Copy link
Contributor

keu commented May 3, 2021

/publish connector=connectors/source-zendesk-chat

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

@keu keu merged commit 6bc6326 into master May 3, 2021
@keu keu deleted the ykurochkin/new-connector-zendesk-chat branch May 3, 2021 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Source Zendesk Chat: Implement Read
4 participants