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 Postgres: Fix not being able to configure wal2json plugin #19985

Merged
merged 11 commits into from
Dec 6, 2022

Conversation

edgao
Copy link
Contributor

@edgao edgao commented Dec 1, 2022

https://github.com/airbytehq/airbyte/pull/18779/files#diff-144a4a0053518cdd47b47a03ce984697ca4081a3d49e8d70505915ab7650dadaL266-R250 accidentally switched the plugin config option to const: pgoutput. Switch it back to default, assuming it still passes SATs.

@edgao

This comment was marked as outdated.

@edgao

This comment was marked as outdated.

@edgao edgao temporarily deployed to more-secrets December 1, 2022 16:12 Inactive
@github-actions
Copy link
Contributor

github-actions bot commented Dec 1, 2022

Affected Connector Report

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to do the following as needed:

  • Run integration tests
  • Bump connector or module version
  • Add changelog
  • Publish the new version

⚠ Sources (3)

Connector Version Changelog Publish
source-alloydb 1.0.17
source-alloydb-strict-encrypt 1.0.17
(not in seed)
source-postgres-strict-encrypt 1.0.30
(not in seed)
  • See "Actionable Items" below for how to resolve warnings and errors.

✅ Destinations (0)

Connector Version Changelog Publish
  • See "Actionable Items" below for how to resolve warnings and errors.

✅ Other Modules (0)

Actionable Items

(click to expand)

Category Status Actionable Item
Version
mismatch
The version of the connector is different from its normal variant. Please bump the version of the connector.

doc not found
The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like source-jdbc is not published or documented). Please double-check to make sure that it is not a bug.
Changelog
doc not found
The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like source-jdbc is not published or documented). Please double-check to make sure that it is not a bug.

changelog missing
There is no chnagelog for the current version of the connector. If you are the author of the current version, please add a changelog.
Publish
not in seed
The connector is not in the seed file (e.g. source_definitions.yaml), so its publication status cannot be checked. This can be normal (e.g. some connectors are cloud-specific, and only listed in the cloud seed file). Please double-check to make sure that it is not a bug.

diff seed version
The connector exists in the seed file, but the latest version is not listed there. This usually means that the latest version is not published. Please use the /publish command to publish the latest version.

@edgao

This comment was marked as outdated.

@edgao edgao temporarily deployed to more-secrets December 1, 2022 17:46 Inactive
@smartpierre
Copy link

Hi, is this why I get these errors on my Postgresql syncs ? If yes, how can I fix ?

2022-12-01 18:31:04 INFO i.a.v.j.JsonSchemaValidator(test):130 - JSON schema validation failed. 
errors: $.method: must be a constant value Standard
2022-12-01 18:31:04 INFO i.a.v.j.JsonSchemaValidator(test):130 - JSON schema validation failed. 
errors: $.plugin: must be a constant value pgoutput
2022-12-01 18:31:04 INFO i.a.v.j.JsonSchemaValidator(test):130 - JSON schema validation failed. 
errors: $.method: does not have a value in the enumeration [Standard]

Here the source configuration in Airbyte database :

{
    "host": "redacted",
    "port": 5432,
    "database": "redacted",
    "password": {
        "_secret": "redacted"
    },
    "username": "airbyte",
    "replication_method": {
        "method": "CDC",
        "plugin": "wal2json",
        "publication": "airbyte_publication_dataupload",
        "replication_slot": "airbyte_replication_slot_dataupload"
    }
}

Best regards, Pierre.

@edgao

This comment was marked as outdated.

@edgao
Copy link
Contributor Author

edgao commented Dec 1, 2022

@smartpierre yeah, that sounds related. As a workaround, I think you can downgrade source-postgres to 1.0.23. But also I'm hoping to get this PR merged in the next couple days, if you're ok with waiting til then.

@edgao edgao temporarily deployed to more-secrets December 1, 2022 22:37 Inactive
@edgao
Copy link
Contributor Author

edgao commented Dec 2, 2022

/test connector=connectors/source-jdbc

🕑 connectors/source-jdbc https://github.com/airbytehq/airbyte/actions/runs/3597791206
✅ connectors/source-jdbc https://github.com/airbytehq/airbyte/actions/runs/3597791206
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                     199     93    53%   35, 41-43, 48, 53, 59, 65, 71, 77-79, 98, 103-105, 111-113, 119-120, 125-126, 131, 137, 146-155, 161-166, 181, 205, 236, 242, 250-255, 263-268, 276-289, 294-300, 307-318, 325-341
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              398    111    72%   53, 58, 87-95, 100-107, 111-112, 116-117, 299, 337-354, 363-371, 375-380, 386, 419-424, 462-469, 512-514, 517, 582-590, 602-605, 610, 666-667, 673, 676, 712-722, 735-760
	 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       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     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1569    350    78%

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 20.74s ==================

@edgao

This comment was marked as outdated.

@edgao
Copy link
Contributor Author

edgao commented Dec 2, 2022

/test connector=connectors/source-mysql-strict-encrypt

🕑 connectors/source-mysql-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/3597793540
✅ connectors/source-mysql-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/3597793540
No Python unittests run

Build Passed

Test summary info:

All Passed

@edgao

This comment was marked as outdated.

@edgao
Copy link
Contributor Author

edgao commented Dec 2, 2022

/test connector=connectors/source-postgres-strict-encrypt

🕑 connectors/source-postgres-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/3597795469
✅ connectors/source-postgres-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/3597795469
No Python unittests run

Build Passed

Test summary info:

All Passed

@edgao
Copy link
Contributor Author

edgao commented Dec 2, 2022

/test connector=connectors/source-snowflake

🕑 connectors/source-snowflake https://github.com/airbytehq/airbyte/actions/runs/3597798535
✅ connectors/source-snowflake https://github.com/airbytehq/airbyte/actions/runs/3597798535
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                     199     93    53%   35, 41-43, 48, 53, 59, 65, 71, 77-79, 98, 103-105, 111-113, 119-120, 125-126, 131, 137, 146-155, 161-166, 181, 205, 236, 242, 250-255, 263-268, 276-289, 294-300, 307-318, 325-341
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              398    111    72%   53, 58, 87-95, 100-107, 111-112, 116-117, 299, 337-354, 363-371, 375-380, 386, 419-424, 462-469, 512-514, 517, 582-590, 602-605, 610, 666-667, 673, 676, 712-722, 735-760
	 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       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     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1569    350    78%

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 22.35s ==================

@edgao edgao temporarily deployed to more-secrets December 2, 2022 00:37 Inactive
@edgao edgao marked this pull request as ready for review December 2, 2022 00:53
@edgao edgao requested a review from a team as a code owner December 2, 2022 00:53
@edgao edgao temporarily deployed to more-secrets December 2, 2022 17:10 Inactive
@octavia-squidington-iv octavia-squidington-iv added the area/documentation Improvements or additions to documentation label Dec 2, 2022
@edgao

This comment was marked as outdated.

@edgao edgao temporarily deployed to more-secrets December 2, 2022 18:37 Inactive
@edgao

This comment was marked as outdated.

@edgao edgao temporarily deployed to more-secrets December 2, 2022 20:50 Inactive
@edgao

This comment was marked as outdated.

@edgao

This comment was marked as outdated.

@edgao edgao temporarily deployed to more-secrets December 5, 2022 19:16 Inactive
@edgao edgao temporarily deployed to more-secrets December 5, 2022 19:17 Inactive
@edgao
Copy link
Contributor Author

edgao commented Dec 5, 2022

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3623810271
✅ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3623810271
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                     199     93    53%   35, 41-43, 48, 53, 59, 65, 71, 77-79, 98, 103-105, 111-113, 119-120, 125-126, 131, 137, 146-155, 161-166, 181, 205, 236, 242, 250-255, 263-268, 276-289, 294-300, 307-318, 325-341
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              398    111    72%   53, 58, 87-95, 100-107, 111-112, 116-117, 299, 337-354, 363-371, 375-380, 386, 419-424, 462-469, 512-514, 517, 582-590, 602-605, 610, 666-667, 673, 676, 712-722, 735-760
	 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       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     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1569    350    78%

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 35.95s ==================

@edgao
Copy link
Contributor Author

edgao commented Dec 5, 2022

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/3623809387

@edgao edgao temporarily deployed to more-secrets December 5, 2022 20:40 Inactive
@edgao edgao temporarily deployed to more-secrets December 5, 2022 20:41 Inactive
@edgao
Copy link
Contributor Author

edgao commented Dec 6, 2022

merging b/c this only affects source-postgres production code. source-mysql has broken tests but they're slightly less broken after this PR. @rodireich will publish this as part of another PR.

@edgao edgao merged commit ea3fb89 into master Dec 6, 2022
@edgao edgao deleted the edgao/source_postgres_cdc_plugin branch December 6, 2022 01:08
@edgao
Copy link
Contributor Author

edgao commented Dec 7, 2022

@smartpierre - source-postgres:1.0.31 has also been released, so you can upgrade to the latest version and it will include this fix.

@smartpierre
Copy link

Hello @edgao we updated airbyte and our connectors, however, now it doesn't block the execution, but I can still see the errors.
Here's the output from one of our connections :

2022-12-19 14:26:17 INFO i.a.v.j.JsonSchemaValidator(test):130 - JSON schema validation failed. 
errors: $.method: must be a constant value Standard
2022-12-19 14:26:17 INFO i.a.v.j.JsonSchemaValidator(test):130 - JSON schema validation failed. 
errors: $.access_token: is missing but it is required, $.refresh_token: is missing but it is required
2022-12-19 14:26:17 INFO i.a.v.j.JsonSchemaValidator(test):130 - JSON schema validation failed. 
errors: $.private_key: is missing but it is required
2022-12-19 14:26:17 INFO i.a.v.j.JsonSchemaValidator(test):130 - JSON schema validation failed. 
errors: $.password: object found, string expected
2022-12-19 14:26:17 INFO i.a.v.j.JsonSchemaValidator(test):130 - JSON schema validation failed. 
errors: $.method: does not have a value in the enumeration [Standard]

Airbyte version : 0.40.26
Postgresql source : 1.0.34
Snowflake destination : 0.4.40

Hope that helps, thanks for the fix 😄

@edgao
Copy link
Contributor Author

edgao commented Dec 19, 2022

Is this causing sync failures? (and can you post the full logs?) I think this is expected (just a misleading log message)

@smartpierre
Copy link

No, the syncs are running properly !

But I was wondering why the log messages are still here. Marking validation errors as INFO are kinda misleading I guess.

@edgao
Copy link
Contributor Author

edgao commented Dec 20, 2022

yeah... maybe makes sense to shift this to debug level

public boolean test(final JsonNode schemaJson, final JsonNode objectJson) {
final Set<ValidationMessage> validationMessages = validateInternal(schemaJson, objectJson);
if (!validationMessages.isEmpty()) {
LOGGER.info("JSON schema validation failed. \nerrors: {}", Strings.join(validationMessages, ", "));
}
return validationMessages.isEmpty();
}

it's generally safe to ignore though - there are a number of cases where schema validation is actually expected to fail (e.g. after a connector spec change)

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.

5 participants