Java connectors use protocol v0 objects #20404

gosusnp · 2022-12-12T23:40:06Z

What

Migrate the Java connectors to use the protocol objects in the v0 namespace (io.airbyte.protocol.models.v0).

Closes #20037

How

Duplicate some helper objects into io.airbyte.protocol.models.v0
Update imports for the Java connectors
Add object conversions in some shared test objects of the Java connectors to keep the build happy

The object conversions is a short term mitigation for the build, a better solution should be implemented when we start migrating some connectors to the v1 of the protocol.

🚨 User Impact 🚨

There should be no behavior change, the v0 and v1 of the protocol are structurally identical, what differs is the interpretation of some fields.

gosusnp · 2022-12-14T21:21:04Z

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3698829907
❌ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3698829907
🐛 https://gradle.com/s/phecw4g25erfw

Build Failed

Test summary info:

Could not find result summary

gosusnp · 2022-12-14T22:20:48Z

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3699213012
❌ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3699213012
🐛 https://gradle.com/s/uk25ye6rfxhgy

Build Failed

Test summary info:

Could not find result summary

gosusnp · 2022-12-14T22:27:28Z

@edgao, @akashkulk, while looking into the connector failure, found extra references to the v-null objects.
Most of the fixes should be purely namespace changes, one to be aware of is AbstractDbSource.java where I added a conversion for the state messages. I think it is the first conversion that is happening in the main code path (as opposed to tests only for the others)

gosusnp · 2022-12-14T22:28:28Z

/test connector=connectors/destination-snowflake

🕑 connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/3699257990
✅ connectors/destination-snowflake https://github.com/airbytehq/airbyte/actions/runs/3699257990
Python tests coverage:

Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
normalization/transform_config/__init__.py                            2      0   100%
normalization/transform_catalog/reserved_keywords.py                 14      0   100%
normalization/transform_catalog/__init__.py                           2      0   100%
normalization/destination_type.py                                    14      0   100%
normalization/__init__.py                                             4      0   100%
normalization/transform_catalog/destination_name_transformer.py     166      8    95%
normalization/transform_catalog/table_name_registry.py              174     34    80%
normalization/transform_config/transform.py                         189     48    75%
normalization/transform_catalog/utils.py                             51     14    73%
normalization/transform_catalog/dbt_macro.py                         22      7    68%
normalization/transform_catalog/catalog_processor.py                147     80    46%
normalization/transform_catalog/transform.py                         61     38    38%
normalization/transform_catalog/stream_processor.py                 595    400    33%
-------------------------------------------------------------------------------------
TOTAL                                                              1441    629    56%

Build Passed

Test summary info:

All Passed

edgao

thanks for the heads up - not expecting anything to break here 🤞hopefully the tests pass

sherifnada · 2022-12-15T00:36:19Z

@gosusnp review is requested from the extensibility team but this doesn't seem to touch any extensibility code. Was that review request intentional?

akashkulk · 2022-12-15T00:38:14Z

/test connector=connectors/source-postgres

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

	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       140      5    96%   87, 93, 238, 242-243
	 source_acceptance_test/conftest.py                     208     92    56%   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-280, 288-301, 306-312, 319-330, 337-353
	 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       158     14    91%   52-59, 64-77, 240
	 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                                                 1603    336    79%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestConnection.test_check: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestDiscovery.test_discover: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestBasicRead.test_read: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestFullRefresh.test_sequential_reads: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
================= 14 passed, 5 skipped, 21 warnings in 32.00s ==================

akashkulk · 2022-12-15T00:38:47Z

Looks good - I assume that was why the Postgres test was failing? Just kicked off another run there

gosusnp · 2022-12-15T00:43:35Z

@sherifnada, that was not intentional, I suspect some change detection script (Check if a review is required from Connector teams maybe?) automatically tagged extensibility

gosusnp · 2022-12-15T00:44:05Z

/test connector=connectors/source-postgres

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

	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       140      5    96%   87, 93, 238, 242-243
	 source_acceptance_test/conftest.py                     208     92    56%   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-280, 288-301, 306-312, 319-330, 337-353
	 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       158     14    91%   52-59, 64-77, 240
	 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                                                 1603    336    79%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestConnection.test_check: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestDiscovery.test_discover: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestBasicRead.test_read: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestFullRefresh.test_sequential_reads: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
================= 14 passed, 5 skipped, 21 warnings in 29.71s ==================

gosusnp · 2022-12-15T00:45:29Z

@akashkulk, I think b16f07d should fix the issues in the source-postgres. Just kicked off another run to confirm this.

gosusnp added 3 commits December 12, 2022 15:38

Dupl v0 objects

35e5457

v0 namespace swap

0014c0a

Introduce some object conversions in the tests

ea51cee

gosusnp temporarily deployed to more-secrets December 12, 2022 23:40 — with GitHub Actions Inactive

gosusnp temporarily deployed to more-secrets December 14, 2022 19:06 — with GitHub Actions Inactive

gosusnp temporarily deployed to more-secrets December 14, 2022 19:07 — with GitHub Actions Inactive

Use v0 objects in db-lib

243b3b1

gosusnp temporarily deployed to more-secrets December 14, 2022 21:11 — with GitHub Actions Inactive

Fix more protocol object references

0d33ee8

octavia-squidington-iv added the connectors/destination/aws-datalake label Dec 14, 2022

gosusnp temporarily deployed to more-secrets December 14, 2022 22:22 — with GitHub Actions Inactive

edgao approved these changes Dec 14, 2022

View reviewed changes

sherifnada removed the request for review from a team December 15, 2022 00:36

Fix arg capture in tests

b16f07d

gosusnp temporarily deployed to more-secrets December 15, 2022 00:42 — with GitHub Actions Inactive

gosusnp temporarily deployed to more-secrets December 15, 2022 00:43 — with GitHub Actions Inactive

Merge branch 'master' into gosusnp/20037-java-connectors-on-protocol-v0

dc799b0

gosusnp temporarily deployed to more-secrets December 15, 2022 19:24 — with GitHub Actions Inactive

gosusnp merged commit 094aff1 into master Dec 15, 2022

gosusnp deleted the gosusnp/20037-java-connectors-on-protocol-v0 branch December 15, 2022 21:39

octavia-squidington-iii mentioned this pull request Dec 18, 2022

Bump Airbyte version from 0.40.25 to 0.40.26 #20630

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Java connectors use protocol v0 objects #20404

Java connectors use protocol v0 objects #20404

gosusnp commented Dec 12, 2022 •

edited

Loading

gosusnp commented Dec 14, 2022 •

edited by github-actions bot

Loading

gosusnp commented Dec 14, 2022 •

edited by github-actions bot

Loading

gosusnp commented Dec 14, 2022

gosusnp commented Dec 14, 2022 •

edited by github-actions bot

Loading

edgao left a comment

sherifnada commented Dec 15, 2022

akashkulk commented Dec 15, 2022 •

edited by github-actions bot

Loading

akashkulk commented Dec 15, 2022

gosusnp commented Dec 15, 2022

gosusnp commented Dec 15, 2022 •

edited by github-actions bot

Loading

gosusnp commented Dec 15, 2022

Java connectors use protocol v0 objects #20404

Java connectors use protocol v0 objects #20404

Conversation

gosusnp commented Dec 12, 2022 • edited Loading

What

How

Recommended reading order

🚨 User Impact 🚨

gosusnp commented Dec 14, 2022 • edited by github-actions bot Loading

Build Failed

gosusnp commented Dec 14, 2022 • edited by github-actions bot Loading

Build Failed

gosusnp commented Dec 14, 2022

gosusnp commented Dec 14, 2022 • edited by github-actions bot Loading

Build Passed

edgao left a comment

Choose a reason for hiding this comment

sherifnada commented Dec 15, 2022

akashkulk commented Dec 15, 2022 • edited by github-actions bot Loading

Build Passed

akashkulk commented Dec 15, 2022

gosusnp commented Dec 15, 2022

gosusnp commented Dec 15, 2022 • edited by github-actions bot Loading

Build Passed

gosusnp commented Dec 15, 2022

gosusnp commented Dec 12, 2022 •

edited

Loading

gosusnp commented Dec 14, 2022 •

edited by github-actions bot

Loading

gosusnp commented Dec 14, 2022 •

edited by github-actions bot

Loading

gosusnp commented Dec 14, 2022 •

edited by github-actions bot

Loading

akashkulk commented Dec 15, 2022 •

edited by github-actions bot

Loading

gosusnp commented Dec 15, 2022 •

edited by github-actions bot

Loading