Cleans and Rebase Error Message Factory PR #16202

ryankfu · 2022-09-01T00:02:27Z

What

Closes PR that had race condition with publishing connector. Bumped connector after rebase on master

Previously work was done by GL in these two PRs:

How

Adds new ConnectionErrorException that will include more information with underlying database error and SQLSTATE codes as well a support message if available

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

No breaking changes, merely updates the Exception thrown so that when it's cause there's now a message including SQLSTATE code and error code to help diagnose the issue as well as documentation to error codes for the main DB sources (Postgres & MySQL)

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New 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 by running the /publish command described here
After the connector is published, connector added to connector index as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Secrets in the connector's spec are annotated with airbyte_secret
Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Code reviews completed
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions

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

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

github-actions · 2022-09-01T00:03:13Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

destination-mongodb-strict-encrypt
source-db2
source-clickhouse-strict-encrypt
destination-cassandra
destination-keen
destination-oracle
source-oracle-strict-encrypt
destination-oracle-strict-encrypt
destination-azure-blob-storage
destination-snowflake
destination-meilisearch
source-e2e-test-cloud
source-jdbc
destination-redis
destination-redshift
source-mysql-strict-encrypt
destination-dynamodb
destination-jdbc
source-bigquery
source-relational-db
destination-clickhouse
destination-pubsub
source-sftp
source-kafka
source-e2e-test
destination-e2e-test
destination-postgres
source-mongodb-strict-encrypt
destination-elasticsearch
destination-bigquery-denormalized
destination-databricks
destination-pulsar
destination-bigquery
destination-rockset
destination-gcs
source-elasticsearch
source-snowflake
source-cockroachdb-strict-encrypt
source-clickhouse
destination-postgres-strict-encrypt
destination-mssql
destination-kinesis
source-tidb
destination-mariadb-columnstore
destination-s3
destination-tidb
destination-csv
destination-clickhouse-strict-encrypt
source-mssql-strict-encrypt
destination-mysql
source-mysql
source-mongodb-v2
destination-scylla
source-cockroachdb
destination-mongodb
source-oracle
source-redshift
source-scaffold-java-jdbc
source-mssql
destination-local-json
destination-kafka
source-postgres
source-db2-strict-encrypt
destination-mqtt
destination-dev-null
source-postgres-strict-encrypt
destination-mysql-strict-encrypt
destination-mssql-strict-encrypt

ryankfu · 2022-09-01T00:05:01Z

/test connector=connectors/source-mysql

ryankfu · 2022-09-01T00:05:14Z

/test connector=connectors/source-postgres

ryankfu · 2022-09-02T05:45:23Z

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/2976879350
❌ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/2976879350
🐛 https://gradle.com/s/widdvxjgs27oa

Build Failed

Test summary info:

Could not find result summary

ryankfu · 2022-09-02T05:45:30Z

/test connector=connectors/source-postgres

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

Build Passed

Test summary info:

All Passed

ryankfu · 2022-09-07T01:52:20Z

/test connector=connectors/source-mysql

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

Build Passed

Test summary info:

All Passed

github-actions · 2022-09-10T00:35:57Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

source-db2
source-tidb
destination-kafka
source-elasticsearch
destination-scylla
destination-clickhouse-strict-encrypt
destination-e2e-test
destination-postgres
source-kafka
source-mongodb-strict-encrypt
source-relational-db
destination-csv
destination-bigquery
destination-mariadb-columnstore
source-postgres
destination-redshift
source-scaffold-java-jdbc
destination-mongodb-strict-encrypt
destination-snowflake
destination-kinesis
destination-tidb
destination-rockset
source-redshift
destination-redis
destination-s3
destination-local-json
destination-dev-null
destination-mongodb
destination-oracle-strict-encrypt
destination-pulsar
source-e2e-test-cloud
destination-dynamodb
source-postgres-strict-encrypt
destination-clickhouse
destination-mssql-strict-encrypt
destination-mssql
destination-azure-blob-storage
source-clickhouse-strict-encrypt
destination-databricks
destination-pubsub
source-jdbc
destination-bigquery-denormalized
source-mssql-strict-encrypt
destination-gcs
destination-elasticsearch
source-sftp
source-snowflake
source-clickhouse
source-mysql
source-alloydb
source-mysql-strict-encrypt
destination-mysql-strict-encrypt
destination-oracle
source-db2-strict-encrypt
source-cockroachdb-strict-encrypt
destination-keen
destination-meilisearch
destination-mqtt
destination-postgres-strict-encrypt
source-bigquery
source-oracle
destination-mysql
source-mongodb-v2
source-mssql
destination-cassandra
destination-jdbc
source-e2e-test
source-cockroachdb
source-oracle-strict-encrypt

github-actions · 2022-09-10T01:22:22Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

destination-cassandra
source-mongodb-v2
source-cockroachdb-strict-encrypt
source-clickhouse-strict-encrypt
destination-rockset
destination-kinesis
source-alloydb
destination-clickhouse
destination-clickhouse-strict-encrypt
destination-postgres-strict-encrypt
destination-e2e-test
destination-csv
source-elasticsearch
source-tidb
destination-redshift
destination-postgres
destination-mssql
destination-keen
source-mysql
source-postgres
source-e2e-test-cloud
destination-meilisearch
source-mysql-strict-encrypt
destination-azure-blob-storage
source-oracle-strict-encrypt
destination-bigquery-denormalized
source-kafka
destination-mongodb
destination-mqtt
destination-redis
destination-pubsub
destination-jdbc
destination-mssql-strict-encrypt
source-snowflake
destination-scylla
source-e2e-test
source-jdbc
destination-tidb
source-mssql-strict-encrypt
destination-mariadb-columnstore
destination-mysql-strict-encrypt
source-redshift
destination-dev-null
destination-kafka
source-postgres-strict-encrypt
source-mssql
source-db2-strict-encrypt
source-cockroachdb
source-relational-db
source-db2
source-bigquery
destination-pulsar
destination-oracle
source-scaffold-java-jdbc
destination-elasticsearch
source-clickhouse
destination-oracle-strict-encrypt
destination-s3
source-sftp
source-oracle
destination-snowflake
destination-gcs
destination-dynamodb
destination-databricks
destination-mongodb-strict-encrypt
destination-local-json
destination-bigquery
destination-mysql
source-mongodb-strict-encrypt

ryankfu · 2022-09-12T16:30:57Z

/test connector=connectors/source-mysql

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

Build Passed

Test summary info:

All Passed

grishick · 2022-09-12T18:15:32Z

Thank you for driving these changes to 🏁 as well as paying attention to small details!

ryankfu · 2022-09-12T18:16:20Z

/publish connector=connectors/source-mysql run-tests=false

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

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

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

ryankfu · 2022-09-12T18:16:52Z

/publish connector=connectors/source-mysql-strict-encrypt auto-bump-version=false

🕑 Publishing the following connectors:
connectors/source-mysql-strict-encrypt
https://github.com/airbytehq/airbyte/actions/runs/3039587650

Connector	Did it publish?	Were definitions generated?
connectors/source-mysql-strict-encrypt	❌	❌

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

ryankfu · 2022-09-12T22:20:37Z

/publish connector=connectors/source-mysql-strict-encrypt auto-bump-version=false run-tests=false

🕑 Publishing the following connectors:
connectors/source-mysql-strict-encrypt
https://github.com/airbytehq/airbyte/actions/runs/3040958286

Connector	Did it publish?	Were definitions generated?
connectors/source-mysql-strict-encrypt	✅	❌

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

* Cleaned error messages factory PR * Bumped MySQL and Postgres version * Fixed messages and typos in test * Fixes the changelog conflict with per-stream state * Added note for flaky test * Bumps mysql version to match changelog * Added exception objects to all LOGGER.error for more visibility * auto-bump connector version [ci skip] Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>

bleonard · 2022-10-06T00:19:41Z

...ion-jdbc/src/main/java/io/airbyte/integrations/destination/jdbc/AbstractJdbcDestination.java

@@ -59,6 +64,12 @@ public AirbyteConnectionStatus check(final JsonNode config) {
      final String outputSchema = namingResolver.getIdentifier(config.get(JdbcUtils.SCHEMA_KEY).asText());
      attemptSQLCreateAndDropTableOperations(outputSchema, database, namingResolver, sqlOperations);
      return new AirbyteConnectionStatus().withStatus(Status.SUCCEEDED);
+    } catch (final ConnectionErrorException ex) {


Can someone teach me about what's happening at this spot? We are throwing ConnectionErrorException from various spots. I assume that means "There is a config error" (which is different than the generic programming errors). So we catch it here.
The difference from this block and the generic one below is:

A different message - that makes sense to me because we can do a better job instructing the user what's going on.

AirbyteTraceMessageUtility.emitConfigErrorTrace(ex, message); - what does this do?

My overall hope is that this ConnectionErrorException ends up not creating a sentry error and the below Exception one does. Is that the case?

Yes, the code throws ConnectionErrorException in areas where a configuration error can arise

AirbyteTraceMessage is used to provide to the surface the stack trace using AirbyteMessage as a delivery vehicle. To be quite frank, this was lesser so a decision made from my end an implementation that was carried from the previous revisions/reviews prior to my work

Agreed, that ConnectionErrorException should ideally not be a sentry error, however how much value will be lost if this is excluded from Sentry? From what I was able to quickly search there's an ability to filter out certain Errors, can this be broadly applied where to filter the ConnectionErrorException into a folder so that it's not lost but rather just limited interaction and no github issues created?

EDIT: Currently ConnectionErrorException ends up in sentry

Is the AirbyteTraceMessageUtility.emitConfigErrorTrace that makes it go to Sentry? If so, do you know why that is not in the general Exception catching? cc @pedroslopez

@bleonard It looks like emitConfigErrorTrace is properly setting the FailureType to config_error - we're not currently doing anything with the tag, but it means we can use this tag to correctly exclude them from triggering alerts or otherwise.

Sentry does have some server-side filtering to prevent the issue from showing up in sentry at all, but it's a feature of their Business plan (i believe we're on the Team plan)

An alternative to sentry's server-side filtering would be in our implementation we can ignore anything that's a config_error failure type and don't send it to sentry. We didn't do this originally to get a sense of whether seeing the kinds of config errors users are facing would be useful.

I think regardless we should still wrap it in a trace message so that the failure reason is properly recorded and shown to users in the right place

* Cleaned error messages factory PR * Bumped MySQL and Postgres version * Fixed messages and typos in test * Fixes the changelog conflict with per-stream state * Added note for flaky test * Bumps mysql version to match changelog * Added exception objects to all LOGGER.error for more visibility * auto-bump connector version [ci skip] Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>

ryankfu requested a review from grishick September 1, 2022 00:02

ryankfu requested a review from a team as a code owner September 1, 2022 00:02

github-actions bot added the area/connectors Connector related issues label Sep 1, 2022

octavia-squidington-iii added connectors/source/mysql-strict-encrypt connectors/source/postgres-strict-encrypt labels Sep 1, 2022

ryankfu temporarily deployed to more-secrets September 2, 2022 05:47 Inactive

ryankfu linked an issue Sep 6, 2022 that may be closed by this pull request

Propagate SQL Error Messages to UI #16377

Closed

ryankfu force-pushed the ryan/clean-error-messages-factory branch from 739c782 to 7aa7a37 Compare September 7, 2022 01:51

ryankfu requested a review from a team as a code owner September 7, 2022 01:51

github-actions bot added the normalization label Sep 7, 2022

ryankfu force-pushed the ryan/clean-error-messages-factory branch from 6ac79c0 to 3040e38 Compare September 10, 2022 00:34

ryankfu temporarily deployed to more-secrets September 10, 2022 00:36 Inactive

ryankfu temporarily deployed to more-secrets September 10, 2022 00:38 Inactive

ryankfu requested a review from grishick September 10, 2022 00:55

Added exception objects to all LOGGER.error for more visibility

d0dd001

ryankfu force-pushed the ryan/clean-error-messages-factory branch from 3040e38 to d0dd001 Compare September 10, 2022 01:21

github-actions bot removed area/worker Related to worker area/platform issues related to the platform labels Sep 10, 2022

ryankfu temporarily deployed to more-secrets September 10, 2022 01:23 Inactive

ryankfu temporarily deployed to more-secrets September 12, 2022 00:58 Inactive

grishick approved these changes Sep 12, 2022

View reviewed changes

auto-bump connector version [ci skip]

e5faf1c

ryankfu merged commit 50a8d03 into master Sep 12, 2022

ryankfu deleted the ryan/clean-error-messages-factory branch September 12, 2022 23:08

ryankfu mentioned this pull request Sep 12, 2022

added error messages factory #15832

Closed

octavia-squidington-iii mentioned this pull request Sep 13, 2022

Bump Airbyte version from 0.40.5 to 0.40.6 #16656

Merged

bleonard reviewed Oct 6, 2022

View reviewed changes

bleonard mentioned this pull request Oct 10, 2022

Improved Database Connector Connection Experience #17592

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleans and Rebase Error Message Factory PR #16202

Cleans and Rebase Error Message Factory PR #16202

ryankfu commented Sep 1, 2022 •

edited

Loading

github-actions bot commented Sep 1, 2022

ryankfu commented Sep 1, 2022

ryankfu commented Sep 1, 2022

ryankfu commented Sep 2, 2022 •

edited by github-actions bot

Loading

ryankfu commented Sep 2, 2022 •

edited by github-actions bot

Loading

ryankfu commented Sep 7, 2022 •

edited by github-actions bot

Loading

github-actions bot commented Sep 10, 2022

github-actions bot commented Sep 10, 2022

ryankfu commented Sep 12, 2022 •

edited by github-actions bot

Loading

grishick commented Sep 12, 2022

ryankfu commented Sep 12, 2022 •

edited by github-actions bot

Loading

ryankfu commented Sep 12, 2022 •

edited by github-actions bot

Loading

ryankfu commented Sep 12, 2022 •

edited by github-actions bot

Loading

bleonard Oct 6, 2022

ryankfu Oct 6, 2022 •

edited

Loading

bleonard Oct 6, 2022

pedroslopez Oct 6, 2022

pedroslopez Oct 6, 2022 •

edited

Loading

pedroslopez Oct 6, 2022

Cleans and Rebase Error Message Factory PR #16202

Cleans and Rebase Error Message Factory PR #16202

Conversation

ryankfu commented Sep 1, 2022 • edited Loading

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

github-actions bot commented Sep 1, 2022

ryankfu commented Sep 1, 2022

ryankfu commented Sep 1, 2022

ryankfu commented Sep 2, 2022 • edited by github-actions bot Loading

Build Failed

ryankfu commented Sep 2, 2022 • edited by github-actions bot Loading

Build Passed

ryankfu commented Sep 7, 2022 • edited by github-actions bot Loading

Build Passed

github-actions bot commented Sep 10, 2022

github-actions bot commented Sep 10, 2022

ryankfu commented Sep 12, 2022 • edited by github-actions bot Loading

Build Passed

grishick commented Sep 12, 2022

ryankfu commented Sep 12, 2022 • edited by github-actions bot Loading

ryankfu commented Sep 12, 2022 • edited by github-actions bot Loading

ryankfu commented Sep 12, 2022 • edited by github-actions bot Loading

bleonard Oct 6, 2022

Choose a reason for hiding this comment

ryankfu Oct 6, 2022 • edited Loading

Choose a reason for hiding this comment

bleonard Oct 6, 2022

Choose a reason for hiding this comment

pedroslopez Oct 6, 2022

Choose a reason for hiding this comment

pedroslopez Oct 6, 2022 • edited Loading

Choose a reason for hiding this comment

pedroslopez Oct 6, 2022

Choose a reason for hiding this comment

ryankfu commented Sep 1, 2022 •

edited

Loading

ryankfu commented Sep 2, 2022 •

edited by github-actions bot

Loading

ryankfu commented Sep 2, 2022 •

edited by github-actions bot

Loading

ryankfu commented Sep 7, 2022 •

edited by github-actions bot

Loading

ryankfu commented Sep 12, 2022 •

edited by github-actions bot

Loading

ryankfu commented Sep 12, 2022 •

edited by github-actions bot

Loading

ryankfu commented Sep 12, 2022 •

edited by github-actions bot

Loading

ryankfu commented Sep 12, 2022 •

edited by github-actions bot

Loading

ryankfu Oct 6, 2022 •

edited

Loading

pedroslopez Oct 6, 2022 •

edited

Loading