-
Notifications
You must be signed in to change notification settings - Fork 2
Tests: Add integration tests with CrateDB #192
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -85,7 +85,7 @@ dependencies = [ | |||||
| "databricks-sqlalchemy<3", | ||||||
| "dataclasses-json<0.7", | ||||||
| "dlt<1.29", | ||||||
| "dlt-cratedb<0.2", | ||||||
| "dlt-cratedb @ git+https://github.com/zerotired/dlt-cratedb.git@fix-dynamic-install", | ||||||
| "duckdb<1.6", | ||||||
| "duckdb-engine<0.18", | ||||||
| "elasticsearch<10", | ||||||
|
|
@@ -126,6 +126,7 @@ dependencies = [ | |||||
| "sqlalchemy-bigquery<2", | ||||||
| "sqlalchemy-cratedb<1", | ||||||
| "sqlalchemy-hana<5", | ||||||
| "sqlalchemy-postgresql-relaxed<0.2", | ||||||
| "sqlalchemy-redshift<2", | ||||||
| "sqlalchemy-spanner<2", | ||||||
| "stripe<16", | ||||||
|
|
@@ -177,7 +178,7 @@ optional-dependencies.test = [ | |||||
| "pytest-asyncio<2", | ||||||
| "pytest-cov<8", | ||||||
| "pytest-xdist[psutil]<4", | ||||||
| "testcontainers[mysql,postgres]>=4.9.1,<4.15", | ||||||
| "testcontainers[cratedb,mysql,postgres] @ git+https://github.com/testcontainers/testcontainers-python.git@main", | ||||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you very much for adding a native CrateDB adapter to Testcontainers for Python. With testcontainers 4.15, we will be able to use it in downstream packages swiftly.
Suggested change
|
||||||
| "types-requests<3", | ||||||
| "verlib2", | ||||||
| ] | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,18 +52,30 @@ def get_output_table(): | |
|
|
||
| res = get_output_table() | ||
| assert len(res) == 3 | ||
| assert res[0] == ("message1",) | ||
| assert res[1] == ("message2",) | ||
| assert res[2] == ("message3",) | ||
| if dest_uri.startswith("cratedb://"): | ||
| messages_db = [res[0][0], res[1][0], res[2][0]] | ||
| assert "message1" in messages_db | ||
| assert "message2" in messages_db | ||
| assert "message3" in messages_db | ||
| else: | ||
| assert res[0] == ("message1",) | ||
| assert res[1] == ("message2",) | ||
| assert res[2] == ("message3",) | ||
|
Comment on lines
-55
to
+63
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to self: Maybe adjust the database query instead to emit ordered results? |
||
|
|
||
| # run again, nothing should be inserted into the output table | ||
| run() | ||
|
|
||
| res = get_output_table() | ||
| assert len(res) == 3 | ||
| assert res[0] == ("message1",) | ||
| assert res[1] == ("message2",) | ||
| assert res[2] == ("message3",) | ||
| if dest_uri.startswith("cratedb://"): | ||
| messages_db = [res[0][0], res[1][0], res[2][0]] | ||
| assert "message1" in messages_db | ||
| assert "message2" in messages_db | ||
| assert "message3" in messages_db | ||
| else: | ||
| assert res[0] == ("message1",) | ||
| assert res[1] == ("message2",) | ||
| assert res[2] == ("message3",) | ||
|
|
||
| # add a new message | ||
| producer.produce(topic, "message4".encode("utf-8")) | ||
|
|
@@ -73,10 +85,17 @@ def get_output_table(): | |
| run() | ||
| res = get_output_table() | ||
| assert len(res) == 4 | ||
| assert res[0] == ("message1",) | ||
| assert res[1] == ("message2",) | ||
| assert res[2] == ("message3",) | ||
| assert res[3] == ("message4",) | ||
| if dest_uri.startswith("cratedb://"): | ||
| messages_db = [res[0][0], res[1][0], res[2][0], res[3][0]] | ||
| assert "message1" in messages_db | ||
| assert "message2" in messages_db | ||
| assert "message3" in messages_db | ||
| assert "message4" in messages_db | ||
| else: | ||
| assert res[0] == ("message1",) | ||
| assert res[1] == ("message2",) | ||
| assert res[2] == ("message3",) | ||
| assert res[3] == ("message4",) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,6 +55,12 @@ def test_mqbridge_kafka_to_db(kafka, dest, topic): | |
| with ThreadPoolExecutor() as executor: | ||
| dest_uri = executor.submit(dest.start).result() | ||
|
|
||
| if dest_uri.startswith("cratedb://"): | ||
| pytest.skip( | ||
| "Fails on CrateDB with `DestinationSchemaTampered`, see " | ||
| "https://github.com/crate/dlt-cratedb/issues/14" | ||
| ) | ||
|
|
||
|
Comment on lines
+58
to
+63
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @florinutz: It looks like even when using the newest updates in the dlt-cratedb repository, including yours, the venerable Like demonstrated here, we had to skip all test cases that are still failing on CrateDB. It would be so sweet to resolve them in one way or another, so CrateDB can become a premium-grade data warehouse within omniload and other applications using dlt. 🙏 |
||
| _produce(kafka, topic, ROWS) | ||
|
|
||
| def run(): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import unittest | ||
|
|
||
| import dlt | ||
| from sqlalchemy.util import classproperty | ||
|
|
||
| from omniload.target.cratedb import CrateDBDestination | ||
| from tests.main.test_targets import GenericSqlDestinationFixture | ||
|
|
||
|
|
||
| class CrateDBDestinationTest(unittest.TestCase, GenericSqlDestinationFixture): | ||
| destination = CrateDBDestination() | ||
|
|
||
| @classproperty | ||
| def expected_class(cls): | ||
| return dlt.destinations.cratedb # ty: ignore[unresolved-attribute] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -121,6 +121,12 @@ def smoke_test(dest_uri, dynamodb, schema: str): | |
| assert result[i][1] == pendulum.parse(dynamodb.data[i]["updated_at"]) | ||
|
|
||
| def append_test(dest_uri, dynamodb, schema: str): | ||
|
|
||
| if dest_uri.startswith("cratedb://"): | ||
| pytest.skip( | ||
| "Fails on CrateDB with twice the amount of expected results: AssertionError: assert 6 == 3" | ||
| ) | ||
|
|
||
|
Comment on lines
123
to
+129
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is another type of error we haven't seen before with dlt and CrateDB. It needs to be investigated. |
||
| dest_table = f"{schema}.dynamodb_{get_random_string(5)}" | ||
|
|
||
| # we run it twice to assert that the data in destination doesn't change | ||
|
|
@@ -145,6 +151,13 @@ def append_test(dest_uri, dynamodb, schema: str): | |
|
|
||
| def incremental_test_factory(strategy): | ||
| def incremental_test(dest_uri, dynamodb, schema: str): | ||
|
|
||
| if dest_uri.startswith("cratedb://"): | ||
| pytest.skip( | ||
| "Fails on CrateDB with `DestinationSchemaTampered`, see " | ||
| "https://github.com/crate/dlt-cratedb/issues/14" | ||
| ) | ||
|
|
||
| dest_table = f"{schema}.dynamodb_{get_random_string(5)}" | ||
|
|
||
| result = invoke_ingest_command( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -362,6 +362,12 @@ def test_mongodb_source(mongodb_function, dest): | |
|
|
||
| dest_uri = dest.start() | ||
|
|
||
| if dest_uri.startswith("cratedb://"): | ||
| pytest.skip( | ||
| 'Fails on CrateDB with `"_id" conflicts with system column`, ' | ||
| "see https://github.com/crate/dlt-cratedb/issues/19" | ||
| ) | ||
|
|
||
|
Comment on lines
+365
to
+370
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently we need to skip the MongoDB -> CrateDB integration tests, because of the venerable /cc @florinutz |
||
| try: | ||
| invoke_ingest_command( | ||
| mongo.get_connection_url(), | ||
|
|
@@ -775,5 +781,11 @@ def incremental_multiple_days(mongo, dest_uri: str): | |
| ) | ||
| def test_mongodb_custom_query(testcase, mongodb_function, dest): | ||
| """Test MongoDB custom aggregation queries""" | ||
| testcase(mongodb_function, dest.start()) | ||
| dest_uri = dest.start() | ||
| if dest_uri.startswith("cratedb://"): | ||
| pytest.skip( | ||
| 'Fails on CrateDB with `"_id" conflicts with system column`, ' | ||
| "see https://github.com/crate/dlt-cratedb/issues/19" | ||
| ) | ||
| testcase(mongodb_function, dest_uri) | ||
| dest.stop() | ||
Uh oh!
There was an error while loading. Please reload this page.
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.
@florinutz: It looks like even when using the newest updates in the dlt-cratedb repository, including yours, the venerable
DestinationSchemaTamperederror is still present across the board.