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: Add virtual-hosted-style option #19006

Conversation

Xingyuan-Chen
Copy link
Contributor

@Xingyuan-Chen Xingyuan-Chen commented Nov 5, 2022

*I add a virtual-hosted-style option for S3 source
*It helps to make S3 source support Non-Amazon S3 endpoint who only support virtual hosted style like Aliyun OSS.
This PR closes #18902

@github-actions github-actions bot added the area/connectors Connector related issues label Nov 5, 2022
@vincentkoc vincentkoc self-assigned this Nov 7, 2022
@vincentkoc vincentkoc self-requested a review November 7, 2022 04:52
@vincentkoc vincentkoc changed the title add virtual-hosted-style option for S3 source Source S3: Add virtual-hosted-style option Nov 7, 2022
@vincentkoc
Copy link
Contributor

@Xingyuan-Chen thanks for splitting the source and destination, I’ll take a look.

@sajarin sajarin added the bounty-S Maintainer program: claimable small bounty PR label Nov 7, 2022
@vincentkoc
Copy link
Contributor

vincentkoc commented Nov 8, 2022

/test connector=connectors/source-s3

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

	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       133      3    98%   87, 93, 230
	 source_acceptance_test/conftest.py                     196     97    51%   35, 41-43, 48, 54, 60, 66, 72-74, 80-95, 100, 105-107, 113-115, 121-122, 127-128, 133, 139, 148-157, 163-168, 232, 238, 244-250, 258-263, 271-284, 289-295, 302-313, 320-336
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              329    106    68%   39, 50-58, 63-70, 74-75, 79-80, 164, 202-219, 228-236, 240-245, 251, 284-289, 327-334, 377-379, 382, 447-455, 484-485, 491, 494, 530-540, 553-578
	 source_acceptance_test/tests/test_incremental.py       145     20    86%   21-23, 29-31, 36-43, 48-61, 224
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  77     10    87%   15-16, 24-30, 64, 67
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/config_migration.py        23     23     0%   5-37
	 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     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1479    376    75%
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/s3file.py                                                  37      0   100%
source_s3/s3_utils.py                                                20      0   100%
source_s3/__init__.py                                                 2      0   100%
source_s3/source.py                                                  27      1    96%
source_s3/source_files_abstract/storagefile.py                       23      1    96%
source_s3/stream.py                                                  43      3    93%
source_s3/source_files_abstract/stream.py                           242     17    93%
source_s3/source_files_abstract/formats/abstract_file_parser.py      39      3    92%
source_s3/source_files_abstract/formats/csv_parser.py                89     20    78%
source_s3/source_files_abstract/file_info.py                         26      8    69%
source_s3/utils.py                                                   31     10    68%
source_s3/source_files_abstract/source.py                            37     14    62%
source_s3/exceptions.py                                              10      4    60%
source_s3/source_files_abstract/spec.py                              44     22    50%
source_s3/source_files_abstract/formats/jsonl_parser.py              45     27    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                                                               861    199    77%
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/s3file.py                                                  37      0   100%
source_s3/s3_utils.py                                                20      0   100%
source_s3/exceptions.py                                              10      0   100%
source_s3/__init__.py                                                 2      0   100%
source_s3/source_files_abstract/formats/jsonl_parser.py              45      1    98%
source_s3/stream.py                                                  43      1    98%
source_s3/source_files_abstract/formats/abstract_file_parser.py      39      1    97%
source_s3/source_files_abstract/formats/parquet_parser.py            64      2    97%
source_s3/source_files_abstract/source.py                            37      2    95%
source_s3/source_files_abstract/formats/avro_parser.py               39      3    92%
source_s3/source_files_abstract/file_info.py                         26      3    88%
source_s3/source_files_abstract/stream.py                           242     40    83%
source_s3/source_files_abstract/formats/csv_parser.py                89     20    78%
source_s3/utils.py                                                   31     10    68%
-------------------------------------------------------------------------------------
TOTAL                                                               861     83    90%

Build Passed

Test summary info:

All Passed

@vincentkoc
Copy link
Contributor

Tests passing, in S3 boto3 client the default is auto for addressing style so this is a safe change. No issues with passing these changes. See https://docs.aws.amazon.com/cli/latest/topic/s3-config.html

addressing_style - Specifies which addressing style to use. This controls if the bucket name is in the hostname or part of the URL. Value values are: path, virtual, and auto. The default value is auto.

@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Nov 8, 2022
@vincentkoc
Copy link
Contributor

vincentkoc commented Nov 8, 2022

/publish connector=connectors/source-s3

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


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 ▶️

@sajarin sajarin merged commit 425cc91 into airbytehq:master Nov 8, 2022
@Xingyuan-Chen Xingyuan-Chen deleted the add-virtual-hosted-style-option-for-S3-source branch November 9, 2022 11:04
akashkulk pushed a commit that referenced this pull request Dec 2, 2022
* add virtual-hosted-style option for S3 source

* update s3 version

* auto-bump connector version

Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.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 bounty bounty-S Maintainer program: claimable small bounty PR community connectors/source/s3
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

add virtual-hosted-style option for S3
5 participants