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 Chat: Implement Incremental sync #3157

Merged
merged 11 commits into from
May 3, 2021
Merged

Source Zendesk Chat: Implement Incremental sync #3157

merged 11 commits into from
May 3, 2021

Conversation

yevhenii-ldv
Copy link
Contributor

What

closes #3112.

Pre-merge Checklist

  • Run integration tests
  • Publish Docker images

Recommended reading order

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

@yevhenii-ldv yevhenii-ldv changed the base branch from master to ykurochkin/new-connector-zendesk-chat April 30, 2021 14:11
@yevhenii-ldv
Copy link
Contributor Author

yevhenii-ldv commented Apr 30, 2021 •
edited by github-actions bot
Loading

/test connector=source-zendesk-chat

🕑 source-zendesk-chat https://github.com/airbytehq/airbyte/actions/runs/799685388
❌ source-zendesk-chat https://github.com/airbytehq/airbyte/actions/runs/799685388

@yevhenii-ldv yevhenii-ldv mentioned this pull request Apr 30, 2021
Merged
2 tasks
@yevhenii-ldv
Copy link
Contributor Author

yevhenii-ldv commented Apr 30, 2021 •
edited by github-actions bot
Loading

/test connector=source-zendesk-chat

🕑 source-zendesk-chat https://github.com/airbytehq/airbyte/actions/runs/799699244
❌ source-zendesk-chat https://github.com/airbytehq/airbyte/actions/runs/799699244

@yevhenii-ldv
Copy link
Contributor Author

yevhenii-ldv commented Apr 30, 2021 •
edited by github-actions bot
Loading

/test connector=source-zendesk-chat

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

keu
keu reviewed Apr 30, 2021
View reviewed changes
airbyte-integrations/connectors/source-zendesk-chat/integration_tests/integration_test.py Outdated
return json.load(f)


@pytest.fixture(name="configured_catalog")
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

keu
keu reviewed Apr 30, 2021
View reviewed changes
airbyte-integrations/connectors/source-zendesk-chat/integration_tests/integration_test.py Outdated
return ConfiguredAirbyteCatalog.parse_file(BASE_DIRECTORY / "sample_files/agent_timeline_catalog.json")


class TestZendeskChatSource:
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need this kind of tests if we use acceptance tests, please check HubSpot repo for example

keu
keu reviewed Apr 30, 2021
View reviewed changes
airbyte-integrations/connectors/source-zendesk-chat/source_zendesk_chat/api.py Outdated
return value


class ZendeskChatTimeIncrementalStream(ZendeskChatBaseIncrementalStream, ABC):
Copy link
Contributor

Choose a reason for hiding this comment

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

there is no need to mention Incremental in the name, also I think it is quite redundant to mention ZendeskChat, but it is up to you

keu
keu requested changes Apr 30, 2021
View reviewed changes
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.

small comments

keu
keu approved these changes Apr 30, 2021
View reviewed changes
Copy link
Contributor

@keu keu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

let's address the acceptance tests issue and the rest is LGTM

sherifnada
sherifnada suggested changes Apr 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.

can you pull master into base branch so the diff is cleaned?

README.md Outdated Show resolved Hide resolved
@yevhenii-ldv
Copy link
Contributor Author

yevhenii-ldv commented Apr 30, 2021 •
edited by github-actions bot
Loading

/test connector=source-zendesk-chat

🕑 source-zendesk-chat https://github.com/airbytehq/airbyte/actions/runs/800377262
❌ source-zendesk-chat https://github.com/airbytehq/airbyte/actions/runs/800377262

@eugene-kulak
Merge remote-tracking branch 'origin/ykurochkin/new-connector-zendesk…
e012ebc 
…-chat' into ykurochkin/zendesk-chat-incremental-sync
@keu keu force-pushed the ykurochkin/zendesk-chat-incremental-sync branch from cbf3f46 to e012ebc Compare May 3, 2021 18:59
@keu keu merged commit e2e9ec5 into ykurochkin/new-connector-zendesk-chat May 3, 2021
@keu keu deleted the ykurochkin/zendesk-chat-incremental-sync branch May 3, 2021 19:00
keu pushed a commit that referenced this pull request May 3, 2021
@yevhenii-ldv @eugene-kulak
🎉 New Source: Zendesk Chat (#3088)
6bc6326 
* Zendesk Chat connector: implement full_refresh sync

* format

* Source Zendesk Chat: Implement Incremental sync (#3157)

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

* register Zendesk Chat as source

Co-authored-by: ykurochkin <y.kurochkin@zazmic.com>
Co-authored-by: Eugene Kulak <kulak.eugene@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sherifnada sherifnada sherifnada requested changes

@keu keu keu approved these changes

@po3na4skld po3na4skld Awaiting requested review from po3na4skld

@vitaliizazmic vitaliizazmic Awaiting requested review from vitaliizazmic

@vovavovavovavova vovavovavovavova Awaiting requested review from vovavovavovavova

@Zirochkaa Zirochkaa Awaiting requested review from Zirochkaa

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Source Zendesk Chat: Implement Incremental sync
4 participants
@yevhenii-ldv @keu @sherifnada @eugene-kulak