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 S3: keep processing but warn if OSError happen #21604

Merged
merged 3 commits into from
Jan 24, 2023

Conversation

roman-yermilov-gl
Copy link
Contributor

What

Handle OSError which means no access to key in a bucket. Warn and keep working on another files which we have an access to

@roman-yermilov-gl
Copy link
Contributor Author

roman-yermilov-gl commented Jan 19, 2023

/test connector=connectors/source-s3

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

	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       141      5    96%   87, 93, 239, 243-244
	 source_acceptance_test/conftest.py                     211     95    55%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-285, 293-306, 311-317, 324-335, 342-358
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              402    115    71%   53, 58, 93-104, 109-116, 120-121, 125-126, 308, 346-363, 376-387, 391-396, 402, 435-440, 478-485, 528-530, 533, 598-606, 618-621, 626, 682-683, 689, 692, 728-738, 751-776
	 source_acceptance_test/tests/test_incremental.py       160     14    91%   58-65, 70-83, 246
	 source_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 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       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1609    339    79%
Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
source_s3/source_files_abstract/formats/parquet_spec.py               9      0   100%
source_s3/source_files_abstract/formats/jsonl_spec.py                13      0   100%
source_s3/source_files_abstract/formats/csv_spec.py                  16      0   100%
source_s3/source_files_abstract/formats/avro_spec.py                  5      0   100%
source_s3/s3_utils.py                                                20      0   100%
source_s3/__init__.py                                                 2      0   100%
source_s3/source_files_abstract/storagefile.py                       23      1    96%
source_s3/stream.py                                                  43      3    93%
source_s3/s3file.py                                                  41      3    93%
source_s3/source_files_abstract/stream.py                           248     20    92%
source_s3/source_files_abstract/formats/abstract_file_parser.py      41      4    90%
source_s3/source.py                                                  27      4    85%
source_s3/source_files_abstract/formats/csv_parser.py               102     30    71%
source_s3/source_files_abstract/file_info.py                         26      8    69%
source_s3/utils.py                                                   31     10    68%
source_s3/exceptions.py                                              10      4    60%
source_s3/source_files_abstract/source.py                            39     16    59%
source_s3/source_files_abstract/spec.py                              44     22    50%
source_s3/source_files_abstract/formats/jsonl_parser.py              53     32    40%
source_s3/source_files_abstract/formats/avro_parser.py               39     25    36%
source_s3/source_files_abstract/formats/parquet_parser.py            64     44    31%
-------------------------------------------------------------------------------------
TOTAL                                                               896    226    75%
Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
source_s3/source_files_abstract/storagefile.py                       23      0   100%
source_s3/source_files_abstract/spec.py                              44      0   100%
source_s3/source_files_abstract/formats/parquet_spec.py               9      0   100%
source_s3/source_files_abstract/formats/jsonl_spec.py                13      0   100%
source_s3/source_files_abstract/formats/csv_spec.py                  16      0   100%
source_s3/source_files_abstract/formats/avro_spec.py                  5      0   100%
source_s3/source.py                                                  27      0   100%
source_s3/exceptions.py                                              10      0   100%
source_s3/__init__.py                                                 2      0   100%
source_s3/source_files_abstract/formats/parquet_parser.py            64      2    97%
source_s3/source_files_abstract/formats/abstract_file_parser.py      41      2    95%
source_s3/stream.py                                                  43      3    93%
source_s3/s3file.py                                                  41      3    93%
source_s3/source_files_abstract/formats/avro_parser.py               39      3    92%
source_s3/source_files_abstract/formats/jsonl_parser.py              53      5    91%
source_s3/source_files_abstract/file_info.py                         26      3    88%
source_s3/source_files_abstract/source.py                            39      5    87%
source_s3/source_files_abstract/stream.py                           248     45    82%
source_s3/source_files_abstract/formats/csv_parser.py               102     20    80%
source_s3/s3_utils.py                                                20      4    80%
source_s3/utils.py                                                   31     10    68%
-------------------------------------------------------------------------------------
TOTAL                                                               896    105    88%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:94: The previous and actual specifications are identical.
SKIPPED [6] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:377: The previous and actual discovered catalogs are identical.
=========== 104 passed, 7 skipped, 31 warnings in 243.06s (0:04:03) ============

@octavia-squidington-iv octavia-squidington-iv added the area/documentation Improvements or additions to documentation label Jan 19, 2023
Comment on lines 68 to 70
self.logger.warn(f"We don't have access to {self.url}. "
f"Check whether key {self.url} exists in `{bucket}` bucket and/or has proper ACL permissions")
raise e
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this produce an AirbyteTraceError message? Just logging to stdout won't help users learn what to do

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @brianjlai maybe you recall the answer - does throwing an exception here end up making a useable AirbyteTraceErrorMessage?

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 does. But in this case I catch it in two possible places where it can be raised and handle it

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, this matches the philosophy that @erohmensing is working on with streams being unavailable - we sync what we can and log an error for the streams we can't reach. 👍

@roman-yermilov-gl
Copy link
Contributor Author

roman-yermilov-gl commented Jan 23, 2023

/publish connector=connectors/source-s3

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


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

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

@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets January 23, 2023 10:48 — with GitHub Actions Inactive
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets January 23, 2023 10:48 — with GitHub Actions Inactive
@github-actions
Copy link
Contributor

Airbyte Code Coverage

There is no coverage information present for the Files changed

Total Project Coverage 26.72% 🍏

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/s3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants