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-sentry] [source-sendgrid] [source-greenhouse] add .yaml to list of file imports in setup.py #15800

Merged
merged 15 commits into from
Aug 23, 2022

Conversation

brianjlai
Copy link
Contributor

What

Addresses https://github.com/airbytehq/oncall/issues/462

We're getting file not found errors on the container on Cloud when we try to run syncs with declarative connectors. This is likely because we don't specify that .yaml files in the working directory are usable by the program. I'm going to test this with one source first, and if it works, then I'll roll it out for the other two.

How

Adds *.yaml to package_data so that it can be used at runtime by the declarative source being executed

@github-actions github-actions bot added the area/connectors Connector related issues label Aug 19, 2022
@brianjlai brianjlai changed the title add .yaml to list of file imports in setup.py [source-sentry] add .yaml to list of file imports in setup.py Aug 19, 2022
@sherifnada
Copy link
Contributor

@brianjlai how does this pass SAT but then fail on cloud? 🤔

@brianjlai
Copy link
Contributor Author

That I'm not sure about. I haven't been able to replicate this locally when I package up a connecter on my own machine either. But it is also the only thing I can think of that could cause the file to not be read, since we copy the whole directory over.

I wouldn't expect this to work in any capacity. Figuring out why can be part of a post-fix retro given that this affects all 3 connectors

@brianjlai
Copy link
Contributor Author

I think this is a pretty safe change to publish and test, given that there are no code changes, and a minimal config adjustment that we have configured for other connectors in prod

@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Aug 19, 2022
@brianjlai
Copy link
Contributor Author

brianjlai commented Aug 19, 2022

/test connector=connectors/source-sentry

🕑 connectors/source-sentry https://github.com/airbytehq/airbyte/actions/runs/2891605693
✅ connectors/source-sentry https://github.com/airbytehq/airbyte/actions/runs/2891605693
Python tests coverage:

	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          10      4    60%   15-18
	 source_acceptance_test/config.py                        83      6    93%   78-80, 84-86
	 source_acceptance_test/conftest.py                     164    164     0%   6-282
	 source_acceptance_test/plugin.py                        48     48     0%   6-104
	 source_acceptance_test/tests/test_core.py              329    111    66%   39, 50-58, 63-70, 74-75, 79-80, 164, 202-219, 228-236, 240-245, 251, 284-289, 327-334, 374-376, 379, 439-448, 477-478, 484, 487, 520-530, 543-568, 573-577
	 source_acceptance_test/tests/test_full_refresh.py       52      2    96%   34, 65
	 source_acceptance_test/tests/test_incremental.py       121     25    79%   21-23, 29-31, 36-43, 48-61, 208-216
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  77     17    78%   15-16, 24-30, 47-54, 64, 67
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       110     48    56%   23-26, 32, 36, 39-64, 67-69, 72-74, 77-79, 82-84, 87-89, 92-110, 144-146
	 source_acceptance_test/utils/json_schema_helper.py     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1321    463    65%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestIncremental.test_two_sequential_reads because not found in the config
======================== 26 passed, 1 skipped in 35.90s ========================

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.

publishing this is fine w me but feels like a total shot in the dark. If the connector works locally then that means the yaml was packaged, no? is the problem an issue of relative paths? Does this connector work on a local instance of airbyte for you?

@brianjlai
Copy link
Contributor Author

brianjlai commented Aug 20, 2022

/test connector=connectors/source-sentry

🕑 connectors/source-sentry https://github.com/airbytehq/airbyte/actions/runs/2893012052
✅ connectors/source-sentry https://github.com/airbytehq/airbyte/actions/runs/2893012052
Python tests coverage:

	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          10      4    60%   15-18
	 source_acceptance_test/config.py                        83      6    93%   78-80, 84-86
	 source_acceptance_test/conftest.py                     164    164     0%   6-282
	 source_acceptance_test/plugin.py                        48     48     0%   6-104
	 source_acceptance_test/tests/test_core.py              329    111    66%   39, 50-58, 63-70, 74-75, 79-80, 164, 202-219, 228-236, 240-245, 251, 284-289, 327-334, 374-376, 379, 439-448, 477-478, 484, 487, 520-530, 543-568, 573-577
	 source_acceptance_test/tests/test_full_refresh.py       52      2    96%   34, 65
	 source_acceptance_test/tests/test_incremental.py       121     25    79%   21-23, 29-31, 36-43, 48-61, 208-216
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  77     17    78%   15-16, 24-30, 47-54, 64, 67
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       110     48    56%   23-26, 32, 36, 39-64, 67-69, 72-74, 77-79, 82-84, 87-89, 92-110, 144-146
	 source_acceptance_test/utils/json_schema_helper.py     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1321    463    65%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestIncremental.test_two_sequential_reads because not found in the config
======================== 26 passed, 1 skipped in 33.86s ========================

@brianjlai
Copy link
Contributor Author

brianjlai commented Aug 20, 2022

/test connector=connectors/source-sendgrid

🕑 connectors/source-sendgrid https://github.com/airbytehq/airbyte/actions/runs/2893012094
✅ connectors/source-sendgrid https://github.com/airbytehq/airbyte/actions/runs/2893012094
Python tests coverage:

Name                          Stmts   Miss  Cover
-------------------------------------------------
source_sendgrid/source.py         4      0   100%
source_sendgrid/__init__.py       2      0   100%
-------------------------------------------------
TOTAL                             6      0   100%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          10      4    60%   15-18
	 source_acceptance_test/config.py                        83      6    93%   78-80, 84-86
	 source_acceptance_test/conftest.py                     164    164     0%   6-282
	 source_acceptance_test/plugin.py                        48     48     0%   6-104
	 source_acceptance_test/tests/test_core.py              329    111    66%   39, 50-58, 63-70, 74-75, 79-80, 164, 202-219, 228-236, 240-245, 251, 284-289, 327-334, 374-376, 379, 439-448, 477-478, 484, 487, 520-530, 543-568, 573-577
	 source_acceptance_test/tests/test_full_refresh.py       52      2    96%   34, 65
	 source_acceptance_test/tests/test_incremental.py       121     25    79%   21-23, 29-31, 36-43, 48-61, 208-216
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  77     17    78%   15-16, 24-30, 47-54, 64, 67
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       110     48    56%   23-26, 32, 36, 39-64, 67-69, 72-74, 77-79, 82-84, 87-89, 92-110, 144-146
	 source_acceptance_test/utils/json_schema_helper.py     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1321    463    65%

Build Passed

Test summary info:

All Passed

@brianjlai
Copy link
Contributor Author

brianjlai commented Aug 20, 2022

/test connector=connectors/source-greenhouse

🕑 connectors/source-greenhouse https://github.com/airbytehq/airbyte/actions/runs/2893013728
❌ connectors/source-greenhouse https://github.com/airbytehq/airbyte/actions/runs/2893013728
🐛 https://gradle.com/s/tpivjpgfo7g7s

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - AssertionError: Stre...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestIncremental.test_two_sequential_reads because not found in the config
============== 1 failed, 38 passed, 1 skipped in 90.61s (0:01:30) ==============

@brianjlai brianjlai changed the title [source-sentry] add .yaml to list of file imports in setup.py [source-sentry] [source-sendgrid] [source-greenhouse] add .yaml to list of file imports in setup.py Aug 20, 2022
@brianjlai
Copy link
Contributor Author

brianjlai commented Aug 20, 2022

/test connector=connectors/source-greenhouse

🕑 connectors/source-greenhouse https://github.com/airbytehq/airbyte/actions/runs/2893055558
❌ connectors/source-greenhouse https://github.com/airbytehq/airbyte/actions/runs/2893055558
🐛 https://gradle.com/s/utpyz7lpkdc6k

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - AssertionError: Stre...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestIncremental.test_two_sequential_reads because not found in the config
============= 1 failed, 38 passed, 1 skipped in 101.28s (0:01:41) ==============

@brianjlai
Copy link
Contributor Author

brianjlai commented Aug 20, 2022

/publish connector=connectors/source-sentry

🕑 Publishing the following connectors:
connectors/source-sentry
https://github.com/airbytehq/airbyte/actions/runs/2893079049


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

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

@brianjlai
Copy link
Contributor Author

brianjlai commented Aug 20, 2022

/test connector=connectors/source-greenhouse

🕑 connectors/source-greenhouse https://github.com/airbytehq/airbyte/actions/runs/2893114364
❌ connectors/source-greenhouse https://github.com/airbytehq/airbyte/actions/runs/2893114364
🐛 https://gradle.com/s/u6ytd2vpuklqg

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - AssertionError: Stre...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestIncremental.test_two_sequential_reads because not found in the config
============= 1 failed, 38 passed, 1 skipped in 100.43s (0:01:40) ==============

@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets August 20, 2022 02:00 Inactive
@brianjlai
Copy link
Contributor Author

brianjlai commented Aug 20, 2022

/test connector=connectors/source-greenhouse

🕑 connectors/source-greenhouse https://github.com/airbytehq/airbyte/actions/runs/2893159999
❌ connectors/source-greenhouse https://github.com/airbytehq/airbyte/actions/runs/2893159999
🐛 https://gradle.com/s/5uykiyhqdqaiy

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - AssertionError: Stre...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestIncremental.test_two_sequential_reads because not found in the config
============= 1 failed, 38 passed, 1 skipped in 100.19s (0:01:40) ==============

@brianjlai brianjlai temporarily deployed to more-secrets August 20, 2022 02:15 Inactive
@brianjlai
Copy link
Contributor Author

brianjlai commented Aug 20, 2022

/publish connector=connectors/source-sendgrid

🕑 Publishing the following connectors:
connectors/source-sendgrid
https://github.com/airbytehq/airbyte/actions/runs/2893275452


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

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

@brianjlai
Copy link
Contributor Author

brianjlai commented Aug 20, 2022

/publish connector=connectors/source-greenhouse

🕑 Publishing the following connectors:
connectors/source-greenhouse
https://github.com/airbytehq/airbyte/actions/runs/2893277850


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

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

@brianjlai brianjlai requested a review from a team as a code owner August 22, 2022 07:35
{"stream":"demographics_questions","data":{"translations":[{"name":"Are you a veteran or active member of the United States Armed Forces? (select one)","language":"en"}],"required":false,"name":"Are you a veteran or active member of the United States Armed Forces? (select one)","id":4015594003,"demographic_question_set_id":4002702003,"answer_type":"multi_value_single_select","active":true},"emitted_at":1660156527389}
{"stream":"demographics_questions","data":{"translations":[{"name":"Do you have a disability or chronic condition (physical, visual, auditory, cognitive, mental, emotional, or other) that substantially limits one or more of your major life activities, including mobility, communication (seeing, hearing, speaking), and learning? (select one)","language":"en"}],"required":false,"name":"Do you have a disability or chronic condition (physical, visual, auditory, cognitive, mental, emotional, or other) that substantially limits one or more of your major life activities, including mobility, communication (seeing, hearing, speaking), and learning? (select one)","id":4015596003,"demographic_question_set_id":4002702003,"answer_type":"multi_value_single_select","active":true},"emitted_at":1660156527389}
{"stream":"demographics_questions","data":{"translations":[{"name":"Do you identify as transgender? (select one)","language":"en"}],"required":false,"name":"Do you identify as transgender? (select one)","id":4015598003,"demographic_question_set_id":4002702003,"answer_type":"multi_value_single_select","active":true},"emitted_at":1660156527389}
{"stream":"demographics_questions","data":{"translations":[{"name":"Are you a veteran or active member of the United States Armed Forces?","language":"en"}],"required":false,"name":"Are you a veteran or active member of the United States Armed Forces?","id":4015594003,"demographic_question_set_id":4002702003,"answer_type":"multi_value_single_select","active":true},"emitted_at":1660156527389}
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'm not sure what exactly happened here, but in the last week greenhouse might've changed their default question vernacular, but it no longer lines up with the expected records from last week. I doubled checked this against making the direct query via Postman and it does not include (select one) in the response so I'm confident this was a change on their end or in our test data

@brianjlai brianjlai temporarily deployed to more-secrets August 22, 2022 07:37 Inactive
@@ -6,7 +6,7 @@
from setuptools import find_packages, setup

MAIN_REQUIREMENTS = [
"airbyte-cdk~=0.1.76",
"airbyte-cdk~=0.1.79",
Copy link
Contributor Author

@brianjlai brianjlai Aug 22, 2022

Choose a reason for hiding this comment

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

technically this doesn't exist yet, but I will merge #15814 and publish 0.1.79 first which contains the fix for retrieving schemas while running from a job

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it now exists as of 8/22/22

@brianjlai
Copy link
Contributor Author

brianjlai commented Aug 23, 2022

/test connector=connectors/source-greenhouse

🕑 connectors/source-greenhouse https://github.com/airbytehq/airbyte/actions/runs/2909160564
❌ connectors/source-greenhouse https://github.com/airbytehq/airbyte/actions/runs/2909160564
🐛 https://gradle.com/s/cwv37jvv6oljo

Build Failed

Test summary info:

	 =========================== short test summary info ============================
	 FAILED unit_tests/test_backward_compatibility.py::test_validate_previous_configs[Nested level: Narrowing a field enum should fail.]
	 
	 Results (271.85s):
	 �[32m     304 passed�[0m
	 �[31m       1 failed�[0m
	          - �[36m�[0mtest_backward_compatibility.py�[0m:965 �[31mtest_validate_previous_configs[Nested level: Narrowing a field enum should fail.]�[0m
	 /actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/source-acceptance-test/.venv/lib/python3.9/site-packages/coverage/control.py:788: CoverageWarning: No data was collected. (no-data-collected)
	   self._warn("No data was collected.", slug="no-data-collected")

> Task :airbyte-integrations:bases:source-acceptance-test:_unitTestCoverage FAILED

Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

See https://docs.gradle.org/7.4/userguide/command_line_interface.html#sec:command_line_warnings
42 actionable tasks: 29 executed, 13 up-to-date

Publishing build scan...
https://gradle.com/s/cwv37jvv6oljo

@brianjlai brianjlai temporarily deployed to more-secrets August 23, 2022 06:44 Inactive
@brianjlai
Copy link
Contributor Author

brianjlai commented Aug 23, 2022

/test connector=connectors/source-greenhouse

🕑 connectors/source-greenhouse https://github.com/airbytehq/airbyte/actions/runs/2909431277
✅ connectors/source-greenhouse https://github.com/airbytehq/airbyte/actions/runs/2909431277
Python tests coverage:

Name                            Stmts   Miss  Cover
---------------------------------------------------
source_greenhouse/source.py         4      0   100%
source_greenhouse/__init__.py       2      0   100%
---------------------------------------------------
TOTAL                               6      0   100%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          10      4    60%   15-18
	 source_acceptance_test/config.py                        83      6    93%   78-80, 84-86
	 source_acceptance_test/conftest.py                     164    164     0%   6-282
	 source_acceptance_test/plugin.py                        48     48     0%   6-104
	 source_acceptance_test/tests/test_core.py              329    111    66%   39, 50-58, 63-70, 74-75, 79-80, 164, 202-219, 228-236, 240-245, 251, 284-289, 327-334, 374-376, 379, 439-448, 477-478, 484, 487, 520-530, 543-568, 573-577
	 source_acceptance_test/tests/test_full_refresh.py       52      2    96%   34, 65
	 source_acceptance_test/tests/test_incremental.py       121     25    79%   21-23, 29-31, 36-43, 48-61, 208-216
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  77     17    78%   15-16, 24-30, 47-54, 64, 67
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       110     48    56%   23-26, 32, 36, 39-64, 67-69, 72-74, 77-79, 82-84, 87-89, 92-110, 144-146
	 source_acceptance_test/utils/json_schema_helper.py     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1322    463    65%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestIncremental.test_two_sequential_reads because not found in the config
=================== 39 passed, 1 skipped in 90.54s (0:01:30) ===================

@brianjlai
Copy link
Contributor Author

brianjlai commented Aug 23, 2022

/publish connector=connectors/source-greenhouse

🕑 Publishing the following connectors:
connectors/source-greenhouse
https://github.com/airbytehq/airbyte/actions/runs/2909627058


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

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

@brianjlai
Copy link
Contributor Author

This latest release of greenhouse includes the fix for both the config and schemas read issues. Version 0.2.9 I will deploy to cloud.

And rather than publish two new images under this same PR that already published, I'm gonna open a new PR to fix the remaining schema read issues by bumping the latest CDK. I validated those changes on a local version of Airbyte. This PR should be all set to merge in.

@brianjlai brianjlai temporarily deployed to more-secrets August 23, 2022 19:57 Inactive
@brianjlai brianjlai merged commit 61af7a4 into master Aug 23, 2022
@brianjlai brianjlai deleted the brian/fix_declarative_sentry branch August 23, 2022 21:46
rodireich pushed a commit that referenced this pull request Aug 25, 2022
…st of file imports in setup.py (#15800)

* add .yaml to list of file imports in setup.py

* bump dockerfile version for sentry

* update sentry changelog

* correct sentry, greenhouse, and sendgrid sources to correctly read configs

* add changelog

* update Dockerfile versions

* auto-bump connector version [ci skip]

* auto-bump connector version [ci skip]

* fix greenhouse SAT tests and update to next version of cdk w/ schema read fix

* auto-bump connector version [ci skip]

Co-authored-by: Octavia Squidington III <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants