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

🐛 Source Zendesk Talk: migrate to CDK #7173

Merged
merged 23 commits into from
Nov 13, 2021
Merged

Conversation

gaart
Copy link
Contributor

@gaart gaart commented Oct 19, 2021

What

Closes #6599 and migrate to CDK implementation

How

Handle the next page link on the last page, upon fixing the initial issue I decided to enable SAT, but this required proper catalog and features of CDK, so I decided to migrate to CDK as well.
bonus features:

  • fixed a few issues with incorrect json schema
  • added caching
  • added correct backoff that use retry-after values
  • improve records layout to avoid data duplication in different streams (see IVR-related streams)

Pre-merge Checklist

Expand the relevant checklist and delete the others.

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
  • Connector version bumped like described here

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
  • Credentials added to Github CI. Instructions.
  • /test connector=connectors/<name> command is passing.
  • New Connector version released on Dockerhub by running the /publish command described here

@github-actions github-actions bot added area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Oct 19, 2021
@gaart
Copy link
Contributor Author

gaart commented Oct 19, 2021

/test connector=connectors/source-zendesk-talk

🕑 connectors/source-zendesk-talk https://github.com/airbytehq/airbyte/actions/runs/1359797632
❌ connectors/source-zendesk-talk https://github.com/airbytehq/airbyte/actions/runs/1359797632
🐛

@gaart gaart linked an issue Oct 19, 2021 that may be closed by this pull request
@jrhizor jrhizor temporarily deployed to more-secrets October 19, 2021 15:19 Inactive
@gaart
Copy link
Contributor Author

gaart commented Oct 31, 2021

/test connector=connectors/source-zendesk-talk

🕑 connectors/source-zendesk-talk https://github.com/airbytehq/airbyte/actions/runs/1404462071
✅ connectors/source-zendesk-talk https://github.com/airbytehq/airbyte/actions/runs/1404462071
Python tests coverage:

	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                       Stmts   Miss  Cover
	 --------------------------------------------------------------
	 base_python/__init__.py                       13      0   100%
	 base_python/catalog_helpers.py                10      6    40%
	 base_python/cdk/__init__.py                    0      0   100%
	 base_python/cdk/abstract_source.py            83     59    29%
	 base_python/cdk/streams/__init__.py            0      0   100%
	 base_python/cdk/streams/auth/__init__.py       0      0   100%
	 base_python/cdk/streams/auth/core.py           8      1    88%
	 base_python/cdk/streams/auth/jwt.py            5      5     0%
	 base_python/cdk/streams/auth/oauth.py         37     26    30%
	 base_python/cdk/streams/auth/token.py          9      4    56%
	 base_python/cdk/streams/core.py               63     32    49%
	 base_python/cdk/streams/exceptions.py         10      2    80%
	 base_python/cdk/streams/http.py               67     33    51%
	 base_python/cdk/streams/rate_limiting.py      30     14    53%
	 base_python/cdk/utils/__init__.py              0      0   100%
	 base_python/cdk/utils/casing.py                4      0   100%
	 base_python/client.py                         56     33    41%
	 base_python/entrypoint.py                     70     56    20%
	 base_python/integration.py                    52     25    52%
	 base_python/logger.py                         33     19    42%
	 base_python/schema_helpers.py                 56     41    27%
	 base_python/source.py                         51     34    33%
	 main_dev.py                                    3      3     0%
	 --------------------------------------------------------------
	 TOTAL                                        660    393    40%
	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                              Stmts   Miss  Cover
	 -----------------------------------------------------
	 main_dev.py                           6      6     0%
	 source_zendesk_talk/__init__.py       2      0   100%
	 source_zendesk_talk/api.py          169     45    73%
	 source_zendesk_talk/client.py        28     11    61%
	 source_zendesk_talk/errors.py         6      0   100%
	 source_zendesk_talk/source.py         4      0   100%
	 -----------------------------------------------------
	 TOTAL                               215     62    71%
	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                              Stmts   Miss  Cover
	 -----------------------------------------------------
	 main_dev.py                           6      6     0%
	 source_zendesk_talk/__init__.py       2      0   100%
	 source_zendesk_talk/api.py          169     44    74%
	 source_zendesk_talk/client.py        28      3    89%
	 source_zendesk_talk/errors.py         6      0   100%
	 source_zendesk_talk/source.py         4      0   100%
	 -----------------------------------------------------
	 TOTAL                               215     53    75%

@gaart gaart temporarily deployed to more-secrets October 31, 2021 10:28 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets October 31, 2021 10:29 Inactive
@harshithmullapudi
Copy link
Contributor

@keu any update on this ?

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ gaart
❌ eugene-kulak
You have signed the CLA already but the status is still pending? Let us recheck it.

@keu keu temporarily deployed to more-secrets November 9, 2021 16:37 Inactive
@keu keu temporarily deployed to more-secrets November 9, 2021 21:04 Inactive
@keu keu temporarily deployed to more-secrets November 9, 2021 22:20 Inactive
@keu keu temporarily deployed to more-secrets November 9, 2021 23:18 Inactive
@keu keu temporarily deployed to more-secrets November 9, 2021 23:54 Inactive
@keu keu temporarily deployed to more-secrets November 10, 2021 00:21 Inactive
@github-actions github-actions bot added the CDK Connector Development Kit label Nov 10, 2021
@keu keu temporarily deployed to more-secrets November 10, 2021 18:08 Inactive
@keu keu changed the title 🐛 Source Zendesk Talk: Fix pagination edge case 🐛 Source Zendesk Talk: migrate to CDK Nov 11, 2021
@keu keu requested a review from sherifnada November 11, 2021 00:13
@keu keu temporarily deployed to more-secrets November 13, 2021 02:06 Inactive
@keu keu temporarily deployed to more-secrets November 13, 2021 02:39 Inactive
@keu keu requested a review from sherifnada November 13, 2021 02:42
@keu
Copy link
Contributor

keu commented Nov 13, 2021

/test connector=source-zendesk-talk

🕑 source-zendesk-talk https://github.com/airbytehq/airbyte/actions/runs/1455549052
❌ source-zendesk-talk https://github.com/airbytehq/airbyte/actions/runs/1455549052
🐛 https://gradle.com/s/oylpeu3i4jucq

@keu keu temporarily deployed to more-secrets November 13, 2021 02:43 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets November 13, 2021 02:45 Inactive
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.

Looks great


assert alive
assert not error
def test_example_method():
Copy link
Contributor

Choose a reason for hiding this comment

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

we can probably remove this file

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, yeah

@keu keu temporarily deployed to more-secrets November 13, 2021 21:18 Inactive
@keu keu temporarily deployed to more-secrets November 13, 2021 21:31 Inactive
@keu
Copy link
Contributor

keu commented Nov 13, 2021

/test connector=source-zendesk-talk

🕑 source-zendesk-talk https://github.com/airbytehq/airbyte/actions/runs/1457411908
✅ source-zendesk-talk https://github.com/airbytehq/airbyte/actions/runs/1457411908
Python tests coverage:

	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                                 Stmts   Miss  Cover
	 ------------------------------------------------------------------------
	 source_acceptance_test/__init__.py                       2      0   100%
	 source_acceptance_test/base.py                          10      4    60%
	 source_acceptance_test/config.py                        75      8    89%
	 source_acceptance_test/conftest.py                     108    108     0%
	 source_acceptance_test/plugin.py                        47     47     0%
	 source_acceptance_test/tests/__init__.py                 4      0   100%
	 source_acceptance_test/tests/test_core.py              200     94    53%
	 source_acceptance_test/tests/test_full_refresh.py       38     27    29%
	 source_acceptance_test/tests/test_incremental.py        69     38    45%
	 source_acceptance_test/utils/__init__.py                 6      0   100%
	 source_acceptance_test/utils/asserts.py                 37      2    95%
	 source_acceptance_test/utils/common.py                  41     24    41%
	 source_acceptance_test/utils/compare.py                 62     25    60%
	 source_acceptance_test/utils/connector_runner.py        82     49    40%
	 source_acceptance_test/utils/json_schema_helper.py     115     14    88%
	 ------------------------------------------------------------------------
	 TOTAL                                                  896    440    51%
	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                              Stmts   Miss  Cover
	 -----------------------------------------------------
	 source_zendesk_talk/__init__.py       2      0   100%
	 source_zendesk_talk/source.py        33     13    61%
	 source_zendesk_talk/streams.py      139     20    86%
	 -----------------------------------------------------
	 TOTAL                               174     33    81%

@keu keu temporarily deployed to more-secrets November 13, 2021 21:35 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets November 13, 2021 21:36 Inactive
@keu
Copy link
Contributor

keu commented Nov 13, 2021

/publish connector=connectors/source-zendesk-talk

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

@jrhizor jrhizor temporarily deployed to more-secrets November 13, 2021 21:44 Inactive
@keu keu merged commit 5a24ef5 into master Nov 13, 2021
@keu keu deleted the gaart/6599-zendesk-pagination branch November 13, 2021 21:59
@keu keu temporarily deployed to more-secrets November 13, 2021 22:00 Inactive
schlattk pushed a commit to schlattk/airbyte that referenced this pull request Jan 4, 2022
* Upd pagination edge case
* migrate to CDK and connect SAT
+ added primary key based on datetime.now
+ added unittests

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
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Zendesk Talk appears to paginate results
7 participants