🚨🚨Source S3: use platform-handled schema evolution by artem1205 · Pull Request #25127 · airbytehq/airbyte

Artem Inzhyyants (artem1205) · 2023-04-12T20:14:11Z

What

Resolving #11930

How

Remove match_target_schema

🚨 User Impact 🚨

_ab_additional_properties will no more be populated, schema refresh needed.

Pre-merge Checklist

Updating a connector

Community member or Airbyter

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
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

…olution instead

Serhii Lazebnyi (lazebnyi)

Do not forget communicate with TCS about breaking changes.

LGTM!

Catherine Noll (clnoll)

I left one code question for you Artem Inzhyyants (@artem1205).

I also saw a note in the issue that was linked:

NOTE: Dependent on [schema evolution being implemented at platform level](https://docs.google.com/document/d/1zJVUQkVy8ZJUcqzM_hYPGijr4pI0Xx6I2eqpagJ2r7E/edit#).

Given that the document that's linked is dated April 2022 it seems like it's probably done, but I'd like to confirm there's no outstanding work blocking this from being merged. cc Jared Rhizor (@jrhizor)

Assuming there are no blockers & that we're handling schema evolution on the platform, is this a breaking change?

Catherine Noll (clnoll) · 2023-04-18T12:48:58Z

-        """
-        compare_columns = [c for c in target_columns if c not in [self.ab_last_mod_col, self.ab_file_name_col]]
-        # check if we're already matching to avoid unnecessary iteration
-        if set(list(record.keys()) + [self.ab_additional_col]) == set(compare_columns):


Can we remove the ab_additional_col attribute from the FileStream object too?

ab_additional_col removed.
I think yes, it may be a breaking changes, as there might be some users, who are still expect their data in ab_additional_col. In this case, They need to make a schema refresh.

Sorry Artem Inzhyyants (@artem1205) I didn't realize that would be a breaking change. WDYT about leaving ab_additional_col for now and do the breaking change separately?

Catherine Noll (@clnoll) ,
The breaking change is exactly about removing this functionality: removing adding "new" columns in ab_additional_col and make it on platform level (by refreshing the schema)

Thanks for clarifying Artem Inzhyyants (@artem1205). Do you know how the platform will handle the changed schema if we're not putting new fields in ab_additional_col ?

Artem Inzhyyants (artem1205) · 2023-04-27T12:43:56Z

Link to Release Playbook: https://docs.google.com/document/d/1z4ubuw0CiDXxuoMlqJjRx4HnH8AantiH2w0VanYFxpQ

Juan (jnr0790) · 2023-05-02T15:42:17Z

Artem Inzhyyants (@artem1205) reminder to get this out today hopefully before 9am PST.

# Conflicts: # airbyte-integrations/connectors/source-s3/integration_tests/integration_test_abstract.py # airbyte-integrations/connectors/source-s3/source_s3/source_files_abstract/stream.py

Artem Inzhyyants (artem1205) · 2023-05-02T15:56:04Z

/test connector=connectors/source-s3

🕑 connectors/source-s3 https://github.com/airbytehq/airbyte/actions/runs/4863220809
❌ connectors/source-s3 https://github.com/airbytehq/airbyte/actions/runs/4863220809
🐛 https://gradle.com/s/n6edy7yemhfei

Build Failed

Test summary info:

	 =========================== short test summary info ============================
	 FAILED unit_tests/test_stream.py::TestIncrementalFileStream::test_get_schema
	 FAILED unit_tests/test_stream.py::TestIncrementalFileStream::test_get_json_schema
	 FAILED unit_tests/test_stream.py::TestIncrementalFileStream::test_schema_no_files
	 �[31m============ �[31m�[1m3 failed�[0m, �[32m161 passed�[0m, �[33m10 warnings�[0m�[31m in 66.59s (0:01:06)�[0m�[31m =============�[0m

Artem Inzhyyants (artem1205) · 2023-05-02T16:50:02Z

/test connector=connectors/source-s3

Artem Inzhyyants (artem1205) · 2023-05-02T16:52:23Z

Juan (@jnr0790) , working on deployment, will publish once API limits will restore https://github.com/airbytehq/airbyte/actions/runs/4863705385/jobs/8671770488

Artem Inzhyyants (artem1205) · 2023-05-02T17:24:48Z

/test connector=connectors/source-s3

🕑 connectors/source-s3 https://github.com/airbytehq/airbyte/actions/runs/4864010324
❌ connectors/source-s3 https://github.com/airbytehq/airbyte/actions/runs/4864010324
🐛 https://gradle.com/s/xgiotz2fntvjo

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - Failed: Stream test:...
FAILED test_core.py::TestBasicRead::test_read[inputs1] - Failed: Stream test:...
FAILED test_core.py::TestBasicRead::test_read[inputs2] - Failed: Stream test:...
FAILED test_core.py::TestBasicRead::test_read[inputs3] - Failed: Stream test:...
FAILED test_core.py::TestBasicRead::test_read[inputs4] - Failed: Stream test:...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
============= 5 failed, 98 passed, 1 skipped in 270.75s (0:04:30) ==============

Artem Inzhyyants (artem1205) · 2023-05-02T18:12:01Z

/test connector=connectors/source-s3

🕑 connectors/source-s3 https://github.com/airbytehq/airbyte/actions/runs/4864394518
❌ connectors/source-s3 https://github.com/airbytehq/airbyte/actions/runs/4864394518
🐛 https://gradle.com/s/szap5ntgrhm2k

Build Failed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
================== 103 passed, 1 skipped in 285.51s (0:04:45) ==================
	 =========================== short test summary info ============================
	 FAILED integration_tests/integration_test.py::TestIncrementalFileStreamS3::test_stream_records[files10-**-True-3-17-expected_schema10-user_schema10-False-False]
	 FAILED integration_tests/integration_test.py::TestIncrementalFileStreamS3::test_stream_records[files15-**-True-num_columns15-num_records15-expected_schema15-None-True-fails15]
	 FAILED integration_tests/integration_test.py::TestIncrementalFileStreamS3::test_stream_records[files16-**-True-num_columns16-num_records16-expected_schema16-None-True-fails16]
	 FAILED integration_tests/integration_test.py::TestIncrementalFileStreamS3::test_stream_records[files17-**-True-num_columns17-num_records17-expected_schema17-user_schema17-True-fails17]
	 �[31m============= �[31m�[1m4 failed�[0m, �[32m17 passed�[0m, �[33m9 warnings�[0m�[31m in 663.37s (0:11:03)�[0m�[31m =============�[0m

Juan (jnr0790) · 2023-05-03T16:36:45Z

Artem Inzhyyants (@artem1205) looks like this didn't go out. can you let me know when it's ready so I can send a new update date email to users about the change

Artem Inzhyyants (artem1205) · 2023-05-04T11:58:24Z

Juan (@jnr0790) , still working on this, need to fix some integration tests after conflict resolving. I'll let you know once it will be finished.

Artem Inzhyyants (artem1205) · 2023-05-04T12:15:14Z

/test connector=connectors/source-s3

🕑 connectors/source-s3 https://github.com/airbytehq/airbyte/actions/runs/4882645340
❌ connectors/source-s3 https://github.com/airbytehq/airbyte/actions/runs/4882645340
🐛 https://gradle.com/s/atnvdhidedmik

Build Failed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
================== 103 passed, 1 skipped in 274.47s (0:04:34) ==================
	 =========================== short test summary info ============================
	 FAILED integration_tests/integration_test.py::TestIncrementalFileStreamS3::test_stream_records[files10-**-True-3-17-expected_schema10-user_schema10-False-False]
	 FAILED integration_tests/integration_test.py::TestIncrementalFileStreamS3::test_stream_records[files15-**-True-num_columns15-num_records15-expected_schema15-None-True-fails15]
	 FAILED integration_tests/integration_test.py::TestIncrementalFileStreamS3::test_stream_records[files16-**-True-num_columns16-num_records16-expected_schema16-None-True-fails16]
	 FAILED integration_tests/integration_test.py::TestIncrementalFileStreamS3::test_stream_records[files17-**-True-num_columns17-num_records17-expected_schema17-user_schema17-True-fails17]
	 �[31m============= �[31m�[1m4 failed�[0m, �[32m17 passed�[0m, �[33m9 warnings�[0m�[31m in 683.82s (0:11:23)�[0m�[31m =============�[0m

Artem Inzhyyants (artem1205) · 2023-05-04T17:36:55Z

/test connector=connectors/source-s3

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

Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
source_s3/source_files_abstract/storagefile.py                       23      0   100%
source_s3/source_files_abstract/spec.py                              55      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                                                  28      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            67      3    96%
source_s3/source_files_abstract/formats/abstract_file_parser.py      44      3    93%
source_s3/s3file.py                                                  41      3    93%
source_s3/stream.py                                                  50      4    92%
source_s3/source_files_abstract/formats/jsonl_parser.py              56      6    89%
source_s3/source_files_abstract/file_info.py                         26      3    88%
source_s3/source_files_abstract/formats/avro_parser.py               47      6    87%
source_s3/source_files_abstract/formats/csv_parser.py               137     18    87%
source_s3/source_files_abstract/stream.py                           198     33    83%
source_s3/source_files_abstract/source.py                            41      7    83%
source_s3/s3_utils.py                                                20      4    80%
source_s3/utils.py                                                   31     10    68%
-------------------------------------------------------------------------------------
TOTAL                                                               919    100    89%
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                                                  50      3    94%
source_s3/s3file.py                                                  41      3    93%
source_s3/source_files_abstract/stream.py                           198     17    91%
source_s3/source_files_abstract/formats/abstract_file_parser.py      44      5    89%
source_s3/source.py                                                  28      4    86%
source_s3/source_files_abstract/file_info.py                         26      8    69%
source_s3/source_files_abstract/formats/csv_parser.py               137     43    69%
source_s3/utils.py                                                   31     10    68%
source_s3/exceptions.py                                              10      4    60%
source_s3/source_files_abstract/source.py                            41     18    56%
source_s3/source_files_abstract/spec.py                              55     31    44%
source_s3/source_files_abstract/formats/jsonl_parser.py              56     33    41%
source_s3/source_files_abstract/formats/parquet_parser.py            67     45    33%
source_s3/source_files_abstract/formats/avro_parser.py               47     32    32%
-------------------------------------------------------------------------------------
TOTAL                                                               919    257    72%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
================== 103 passed, 1 skipped in 281.53s (0:04:41) ==================

Artem Inzhyyants (artem1205) · 2023-05-04T18:08:19Z

Juan (@jnr0790), all tests passed successfully. I'm ready to merge it.

Juan (jnr0790) · 2023-05-08T14:51:12Z

Artem Inzhyyants (@artem1205) lets merge it Monday May 15th 11am EST. I'll be sending an updated date comms to users

# Conflicts: # airbyte-integrations/connectors/source-s3/Dockerfile # docs/integrations/sources/s3.md

Juan (jnr0790) · 2023-05-15T13:34:40Z

Artem Inzhyyants (@artem1205) just a reminder to get this out today in about 1hr 30mins.

Artem Inzhyyants (artem1205) · 2023-05-15T13:36:04Z

/publish connector=connectors/source-s3

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

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

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

# Conflicts: # airbyte-integrations/connectors/source-s3/metadata.yaml

Artem Inzhyyants (artem1205) · 2023-05-15T14:13:54Z

/publish connector=connectors/source-s3

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

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

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

* Source S3: Remove match_target_schema; use platform-handled schema evolution instead * Source S3: Remove ab_additional_col * Source S3: update docs; bump version * Source S3: fix unit tests * Source S3: fix expected_records * Source S3: revert _match_target_schema * Source S3: update expected records for parquet dataset * Source S3: update metadata * auto-bump connector version --------- Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>

Source S3: Remove match_target_schema; use platform-handled schema ev…

8d8fc54

…olution instead

Octavia Squidington III (octavia-squidington-iii) added area/connectors Connector related issues connectors/source/s3 labels Apr 12, 2023

Artem Inzhyyants (artem1205) requested review from Arsen Losenko (arsenlosenko) and Serhii Lazebnyi (lazebnyi) April 14, 2023 16:32

Serhii Lazebnyi (lazebnyi) requested a review from Catherine Noll (clnoll) April 18, 2023 11:11

Serhii Lazebnyi (lazebnyi) approved these changes Apr 18, 2023

View reviewed changes

Catherine Noll (clnoll) reviewed Apr 18, 2023

View reviewed changes

Artem Inzhyyants (artem1205) added 2 commits April 18, 2023 19:02

Source S3: Remove ab_additional_col

381bdef

Merge branch 'master' into artem1205/source-s3-schema-evolution-11930

39ed2f5

Artem Inzhyyants (artem1205) requested a review from Catherine Noll (clnoll) April 24, 2023 17:00

Artem Inzhyyants (artem1205) added the breaking-change Don't merge me unless you are ready. label Apr 24, 2023

Artem Inzhyyants (artem1205) mentioned this pull request Apr 25, 2023

Source Abstract Files / S3 : Align with platform-handled schema evolution #11930

Closed

Artem Inzhyyants (artem1205) added 3 commits May 2, 2023 17:46

Merge branch 'master' into artem1205/source-s3-schema-evolution-11930

d3214ca

Merge branch 'master' into artem1205/source-s3-schema-evolution-11930

c052aa8

# Conflicts: # airbyte-integrations/connectors/source-s3/integration_tests/integration_test_abstract.py # airbyte-integrations/connectors/source-s3/source_s3/source_files_abstract/stream.py

Source S3: update docs; bump version

2c8c2b5

Octavia Squidington III (octavia-squidington-iii) added the area/documentation Improvements or additions to documentation label May 2, 2023

Source S3: fix unit tests

7ed7862

Source S3: fix expected_records

4984cbe

Source S3: revert _match_target_schema

28c3942

Artem Inzhyyants (artem1205) added 2 commits May 10, 2023 12:35

Merge branch 'master' into artem1205/source-s3-schema-evolution-11930

db9936f

# Conflicts: # airbyte-integrations/connectors/source-s3/Dockerfile # docs/integrations/sources/s3.md

Source S3: update expected records for parquet dataset

4277b98

Artem Inzhyyants (artem1205) added 2 commits May 15, 2023 16:12

Source S3: update metadata

1cd23e9

Merge branch 'master' into artem1205/source-s3-schema-evolution-11930

d5b3755

# Conflicts: # airbyte-integrations/connectors/source-s3/metadata.yaml

auto-bump connector version

a66c28a

Artem Inzhyyants (artem1205) merged commit 93f3286 into master May 15, 2023

Artem Inzhyyants (artem1205) deleted the artem1205/source-s3-schema-evolution-11930 branch May 15, 2023 15:14

Conversation

Artem Inzhyyants (artem1205) commented Apr 12, 2023

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Uh oh!

Serhii Lazebnyi (lazebnyi) left a comment

Choose a reason for hiding this comment

Uh oh!

Catherine Noll (clnoll) left a comment

Choose a reason for hiding this comment

Uh oh!

Catherine Noll (clnoll) Apr 18, 2023

Choose a reason for hiding this comment

Uh oh!

Artem Inzhyyants (artem1205) Apr 18, 2023

Choose a reason for hiding this comment

Uh oh!

Catherine Noll (clnoll) Apr 25, 2023

Choose a reason for hiding this comment

Uh oh!

Artem Inzhyyants (artem1205) Apr 25, 2023

Choose a reason for hiding this comment

Uh oh!

Catherine Noll (clnoll) Apr 26, 2023

Choose a reason for hiding this comment

Uh oh!

Artem Inzhyyants (artem1205) commented Apr 27, 2023

Uh oh!

Juan (jnr0790) commented May 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Artem Inzhyyants (artem1205) commented May 2, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Failed

Uh oh!

Artem Inzhyyants (artem1205) commented May 2, 2023

Uh oh!

Artem Inzhyyants (artem1205) commented May 2, 2023

Uh oh!

Artem Inzhyyants (artem1205) commented May 2, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Failed

Uh oh!

Artem Inzhyyants (artem1205) commented May 2, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Failed

Uh oh!

Juan (jnr0790) commented May 3, 2023

Uh oh!

Artem Inzhyyants (artem1205) commented May 4, 2023

Uh oh!

Artem Inzhyyants (artem1205) commented May 4, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Failed

Uh oh!

Artem Inzhyyants (artem1205) commented May 4, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Passed

Uh oh!

Artem Inzhyyants (artem1205) commented May 4, 2023

Uh oh!

Juan (jnr0790) commented May 8, 2023

Uh oh!

Juan (jnr0790) commented May 15, 2023

Uh oh!

Artem Inzhyyants (artem1205) commented May 15, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Artem Inzhyyants (artem1205) commented May 15, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Juan (jnr0790) commented May 2, 2023 •

edited

Loading

Artem Inzhyyants (artem1205) commented May 2, 2023 •

edited by github-actions Bot

Loading

Artem Inzhyyants (artem1205) commented May 2, 2023 •

edited by github-actions Bot

Loading

Artem Inzhyyants (artem1205) commented May 2, 2023 •

edited by github-actions Bot

Loading

Artem Inzhyyants (artem1205) commented May 4, 2023 •

edited by github-actions Bot

Loading

Artem Inzhyyants (artem1205) commented May 4, 2023 •

edited by github-actions Bot

Loading

Artem Inzhyyants (artem1205) commented May 15, 2023 •

edited by github-actions Bot

Loading

Artem Inzhyyants (artem1205) commented May 15, 2023 •

edited by github-actions Bot

Loading