-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
test for behavior when a column is removed in an incremental sync #20033
Conversation
Affected Connector ReportNOTE
|
Connector | Version | Changelog | Publish |
---|
- See "Actionable Items" below for how to resolve warnings and errors.
❌ Destinations (47)
Connector | Version | Changelog | Publish |
---|---|---|---|
destination-aws-datalake |
0.1.1 |
✅ | ✅ |
destination-azure-blob-storage |
0.1.6 |
✅ | ✅ |
destination-bigquery |
1.2.9 |
✅ | ✅ |
destination-bigquery-denormalized |
1.2.10 |
✅ | ✅ |
destination-cassandra |
0.1.4 |
✅ | ✅ |
destination-clickhouse |
0.2.1 |
✅ | ✅ |
destination-clickhouse-strict-encrypt |
0.2.1 |
🔵 (ignored) |
🔵 (ignored) |
destination-csv |
0.2.10 |
✅ | ✅ |
destination-databricks |
0.3.1 |
✅ | ✅ |
destination-dev-null |
0.2.7 |
🔵 (ignored) |
🔵 (ignored) |
destination-doris |
0.1.0 |
✅ | ✅ |
destination-dynamodb |
0.1.7 |
✅ | ✅ |
destination-e2e-test |
0.2.4 |
✅ | ✅ |
destination-elasticsearch |
0.1.6 |
✅ | ✅ |
destination-elasticsearch-strict-encrypt |
0.1.6 |
🔵 (ignored) |
🔵 (ignored) |
destination-gcs |
0.2.12 |
✅ | ✅ |
destination-iceberg |
0.1.0 |
✅ | ✅ |
destination-jdbc |
0.3.14 |
🔵 (ignored) |
🔵 (ignored) |
destination-kafka |
0.1.10 |
✅ | ✅ |
destination-keen |
0.2.4 |
✅ | ✅ |
destination-kinesis |
0.1.5 |
✅ | ✅ |
destination-local-json |
0.2.11 |
✅ | ✅ |
destination-mariadb-columnstore |
0.1.7 |
✅ | ✅ |
destination-mongodb |
0.1.9 |
✅ | ✅ |
destination-mongodb-strict-encrypt |
0.1.9 |
🔵 (ignored) |
🔵 (ignored) |
destination-mqtt |
0.1.3 |
✅ | ✅ |
destination-mssql |
0.1.22 |
✅ | ✅ |
destination-mssql-strict-encrypt |
0.1.22 |
🔵 (ignored) |
🔵 (ignored) |
destination-mysql |
0.1.20 |
✅ | ✅ |
destination-mysql-strict-encrypt |
❌ 0.1.21 (mismatch: 0.1.20 ) |
🔵 (ignored) |
🔵 (ignored) |
destination-oracle |
0.1.19 |
✅ | ✅ |
destination-oracle-strict-encrypt |
0.1.19 |
🔵 (ignored) |
🔵 (ignored) |
destination-postgres |
0.3.26 |
✅ | ✅ |
destination-postgres-strict-encrypt |
0.3.26 |
🔵 (ignored) |
🔵 (ignored) |
destination-pubsub |
0.2.0 |
✅ | ✅ |
destination-pulsar |
0.1.3 |
✅ | ✅ |
destination-r2 |
0.1.0 |
✅ | ✅ |
destination-redis |
0.1.4 |
✅ | ✅ |
destination-redpanda |
0.1.0 |
✅ | ✅ |
destination-redshift |
0.3.53 |
✅ | ✅ |
destination-rockset |
0.1.4 |
✅ | ✅ |
destination-s3 |
0.3.18 |
✅ | ✅ |
destination-s3-glue |
0.1.1 |
✅ | ✅ |
destination-scylla |
0.1.3 |
✅ | ✅ |
destination-snowflake |
0.4.41 |
✅ | ✅ |
destination-tidb |
0.1.0 |
✅ | ✅ |
destination-yugabytedb |
0.1.0 |
✅ | ✅ |
- 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. |
// remove one field | ||
final JsonNode jsonSchema = configuredCatalog.getStreams().get(0).getStream().getJsonSchema(); | ||
((ObjectNode) jsonSchema.findValue("properties")).remove("HKD"); | ||
configuredCatalog.getStreams().get(0).getStream().setJsonSchema(jsonSchema); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: this isn't technically necessary, since you're modifying the ObjectNode in-place
@@ -561,6 +564,7 @@ public void specDBTValueShouldBeCorrect() throws WorkerException { | |||
* append records to the datastore instead of overwriting the previous run. | |||
*/ | |||
@Test | |||
@Disabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note - don't forget that these need to be un-disabled!
/test connector=destination-snowflake
Build FailedTest summary info:
|
/test connector=destination-bigquery
Build FailedTest summary info:
|
/test connector=destination-bigquery-denormalized
Build PassedTest summary info:
|
/test connector=destination-aws-datalake
Build FailedTest summary info:
|
/test connector=destination-databricks
Build PassedTest summary info:
|
/test connector=destination-s3
Build PassedTest summary info:
|
/test connector=destination-postgres
Build FailedTest summary info:
|
/test connector=destination-postgres
|
/test connector=destination-aws-datalake
|
/test connector=destination-bigquery
|
/test connector=destination-snowflake
|
/test connector=destination-postgres
Build FailedTest summary info:
Build FailedTest summary info:
|
/test connector=destination-aws-datalake
Build FailedTest summary info:
|
/test connector=destination-bigquery
Build FailedTest summary info:
|
/test connector=destination-snowflake
Build FailedTest summary info:
|
/test connector=destination-postgres
Build FailedTest summary info:
|
/test connector=destination-postgres |
/test connector=destination-postgres
Build PassedTest summary info:
|
/test connector=destination-snowflake
Build PassedTest summary info:
|
/test connector=destination-bigquery
Build PassedTest summary info:
|
/test connector=destination-aws-datalake
Build FailedTest summary info:
|
/test connector=destination-aws-datalake
Build FailedTest summary info:
|
/test connector=destination-aws-datalake
Build FailedTest summary info:
|
Quick summary: passing tests for:
No green test for AWS Datalake but from the test run, the actual new test case is passing and it appears that the failure is an unrelated test dating back to July, so I think we can ignore that one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good! one comment about simplifying the test structure but once that's resolved
final AirbyteCatalog catalog = Jsons.deserialize(MoreResources.readResource(catalogFilename), | ||
AirbyteCatalog.class); | ||
|
||
if (!catalog.getStreams().get(0).getName().equals("exchange_rate")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rather than using @ParameterizedTest
, you can just reference DataArgumentsProvider.EXCHANGE_RATE_CONFIG
directly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mfsiega-airbyte what about this comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davinchia me wanting to but not getting a chance to spend the time to address this comment is why this PR isn't already merged :) it's not a trivial context switch, so I haven't found time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this entire if statement can be removed now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll send a follow-up with that cleanup.
@@ -1462,7 +1529,6 @@ public String toString() { | |||
* your_containers_id" (ex. docker container attach 18cc929f44c8) to see the container's output | |||
*/ | |||
@Test | |||
@Disabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enabling this test may break some connectors and will keep lots of trash in DB. Basically, it wasn't even a test, but just a script for manual load emulation.
…0033) * test for behavior when a column is removed in an incremental sync * fixes in dat test for dropping a column * only run drop-one-column test for the exchange rates dataset * re-enable tests that were disabled during development * remove unused import * update test to new method for checking spec capabilities * use config directly instead of parameterized test Co-authored-by: Michael Siega <michael@airbyte.io> Co-authored-by: Michael Siega <109092231+mfsiega-airbyte@users.noreply.github.com>
What
Adds a destination acceptance test for removing a column in an incremental sync.
The goal is to verify that it's safe to go ahead with column selection, particularly for some concerning destinations.