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 Support: do not use nonpublic API #19432

Merged

Conversation

davydov-d
Copy link
Collaborator

What

https://github.com/airbytehq/oncall/issues/1010

How

Remove call to undocumented (nonpublic) API, use a test read instead

@octavia-squidington-iv octavia-squidington-iv added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/zendesk-support labels Nov 15, 2022
@davydov-d
Copy link
Collaborator Author

davydov-d commented Nov 15, 2022

/test connector=connectors/source-zendesk-support

🕑 connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/3471695888
❌ connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/3471695888
🐛 https://gradle.com/s/6fjtj6auqpxr2

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestSpec::test_secret_is_properly_marked[inputs0] - Fail...
=================== 1 failed, 40 passed in 624.52s (0:10:24) ===================

@davydov-d
Copy link
Collaborator Author

blocked by #19465

@davydov-d
Copy link
Collaborator Author

davydov-d commented Nov 16, 2022

/test connector=connectors/source-zendesk-support

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

Name                                 Stmts   Miss  Cover
--------------------------------------------------------
source_zendesk_support/__init__.py       2      0   100%
source_zendesk_support/streams.py      333     34    90%
source_zendesk_support/source.py        54      6    89%
--------------------------------------------------------
TOTAL                                  389     40    90%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       139      5    96%   87, 93, 235, 239-240
	 source_acceptance_test/conftest.py                     196     92    53%   35, 41-43, 48, 54, 60, 66, 72-74, 93, 98-100, 106-108, 114-115, 120-121, 126, 132, 141-150, 156-161, 176, 200, 231, 237, 243-248, 256-261, 269-282, 287-293, 300-311, 318-334
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              398    111    72%   53, 58, 87-95, 100-107, 111-112, 116-117, 299, 337-354, 363-371, 375-380, 386, 419-424, 462-469, 512-514, 517, 582-590, 602-605, 610, 666-667, 673, 676, 712-722, 735-760
	 source_acceptance_test/tests/test_incremental.py       158     14    91%   52-59, 64-77, 240
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       112     50    55%   23-26, 32, 36, 39-68, 71-73, 76-78, 81-83, 86-88, 91-93, 96-114, 148-150
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1563    349    78%

Build Passed

Test summary info:

All Passed

@bazarnov
Copy link
Collaborator

@davydov-d
Please note, with this change the connector will not be able to check the permissions for certain streams, based on the user's subscription plan. Thus, we should be ready to face with even more issues, rather than 422 errors.

@davydov-d
Copy link
Collaborator Author

davydov-d commented Nov 21, 2022

/test connector=connectors/source-zendesk-support

🕑 connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/3513249044
❌ connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/3513249044
🐛 https://gradle.com/s/l4ncwdx2pyeu4

Build Failed

Test summary info:

	 =========================== short test summary info ============================
	 FAILED unit_tests/unit_test.py::test_full_access_streams[forms_inaccessible]
	 �[31m================= �[31m�[1m1 failed�[0m, �[32m155 passed�[0m, �[33m968 warnings�[0m�[31m in 3.57s�[0m�[31m ==================�[0m

@davydov-d
Copy link
Collaborator Author

@davydov-d Please note, with this change the connector will not be able to check the permissions for certain streams, based on the user's subscription plan. Thus, we should be ready to face with even more issues, rather than 422 errors.

yep, but we can not rely on the non public API cause it is not documented and provides no guarantees as well :(

@davydov-d
Copy link
Collaborator Author

davydov-d commented Nov 21, 2022

/test connector=connectors/source-zendesk-support

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

Name                                 Stmts   Miss  Cover
--------------------------------------------------------
source_zendesk_support/__init__.py       2      0   100%
source_zendesk_support/streams.py      333     34    90%
source_zendesk_support/source.py        56      6    89%
--------------------------------------------------------
TOTAL                                  391     40    90%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       139      5    96%   87, 93, 235, 239-240
	 source_acceptance_test/conftest.py                     196     92    53%   35, 41-43, 48, 54, 60, 66, 72-74, 93, 98-100, 106-108, 114-115, 120-121, 126, 132, 141-150, 156-161, 176, 200, 231, 237, 243-248, 256-261, 269-282, 287-293, 300-311, 318-334
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              398    111    72%   53, 58, 87-95, 100-107, 111-112, 116-117, 299, 337-354, 363-371, 375-380, 386, 419-424, 462-469, 512-514, 517, 582-590, 602-605, 610, 666-667, 673, 676, 712-722, 735-760
	 source_acceptance_test/tests/test_incremental.py       158     14    91%   52-59, 64-77, 240
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       112     50    55%   23-26, 32, 36, 39-68, 71-73, 76-78, 81-83, 86-88, 91-93, 96-114, 148-150
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1563    349    78%

Build Passed

Test summary info:

All Passed

@davydov-d
Copy link
Collaborator Author

/test connector=connectors/source-zendesk-support

@davydov-d
Copy link
Collaborator Author

davydov-d commented Nov 28, 2022

/test connector=connectors/source-zendesk-support

🕑 connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/3563207198
❌ connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/3563207198
🐛 https://gradle.com/s/soznbit5hedse

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - Failed: Stream ticke...
=================== 1 failed, 40 passed in 612.59s (0:10:12) ===================

@davydov-d
Copy link
Collaborator Author

davydov-d commented Nov 29, 2022

/test connector=connectors/source-zendesk-support

🕑 connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/3572804946
❌ connectors/source-zendesk-support https://github.com/airbytehq/airbyte/actions/runs/3572804946
🐛 https://gradle.com/s/tvbf7zzlh622y

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - Failed: Stream ticke...
=================== 1 failed, 40 passed in 601.78s (0:10:01) ===================

@davydov-d
Copy link
Collaborator Author

davydov-d commented Nov 29, 2022

/test connector=connectors/source-zendesk-support

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

Name                                 Stmts   Miss  Cover
--------------------------------------------------------
source_zendesk_support/__init__.py       2      0   100%
source_zendesk_support/streams.py      338     35    90%
source_zendesk_support/source.py        56      6    89%
--------------------------------------------------------
TOTAL                                  396     41    90%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       139      5    96%   87, 93, 235, 239-240
	 source_acceptance_test/conftest.py                     196     92    53%   35, 41-43, 48, 54, 60, 66, 72-74, 93, 98-100, 106-108, 114-115, 120-121, 126, 132, 141-150, 156-161, 176, 200, 231, 237, 243-248, 256-261, 269-282, 287-293, 300-311, 318-334
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              398    111    72%   53, 58, 87-95, 100-107, 111-112, 116-117, 299, 337-354, 363-371, 375-380, 386, 419-424, 462-469, 512-514, 517, 582-590, 602-605, 610, 666-667, 673, 676, 712-722, 735-760
	 source_acceptance_test/tests/test_incremental.py       158     14    91%   52-59, 64-77, 240
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       112     50    55%   23-26, 32, 36, 39-68, 71-73, 76-78, 81-83, 86-88, 91-93, 96-114, 148-150
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1563    349    78%

Build Passed

Test summary info:

All Passed

@davydov-d davydov-d requested a review from a team November 29, 2022 10:57
streams.append(ticket_forms_stream)
break
except Exception as e:
logger.warning(f"An exception occurred while trying to access TicketForms stream: {str(e)}. Skipping this stream.")
Copy link
Contributor

Choose a reason for hiding this comment

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

cc @erohmensing FYI, another connector implementing custom stream skipping logic.

Copy link
Contributor

@alafanechere alafanechere left a comment

Choose a reason for hiding this comment

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

Thanks for your effort in setting hightest strictness level without any bypass_reason 👏 .

Comment on lines +131 to +133
# TicketForms stream is only available for Enterprise Plan users but Zendesk API does not provide
# a public API to get user's subscription plan. That's why we try to read at least one record and expose this stream
# in case of success or skip it otherwise
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Comment on lines +135 to +138
for stream_slice in ticket_forms_stream.stream_slices(sync_mode=SyncMode.full_refresh):
for _ in ticket_forms_stream.read_records(sync_mode=SyncMode.full_refresh, stream_slice=stream_slice):
streams.append(ticket_forms_stream)
break
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be useful to store the inferred plan (Enterprise Plan) in the configuration?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean exposing a new field in the config.json?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know exactly. Just a way for the connector to be aware of the inferred plan; wondering if this could be useful for other logic.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well we could possibly store it but as for now we do not have a way to get a plan and be 100% sure it's correct

@davydov-d
Copy link
Collaborator Author

davydov-d commented Nov 29, 2022

/publish connector=connectors/source-zendesk-support

🕑 Publishing the following connectors:
connectors/source-zendesk-support
https://github.com/airbytehq/airbyte/actions/runs/3576516306


Connector Did it publish? Were definitions generated?
connectors/source-zendesk-support

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

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 connectors/source/zendesk-support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants