Fix(ingestion/clickhouse) move to two tier sqlalchemy #8300

alplatonov · 2023-06-26T09:49:06Z

Checklist

[+] The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
[+] Links to related issues Ingest Clickhouse show None database #6544
[+] Tests for the changes have been added/updated (if applicable)
[+] Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
[+] For any breaking change/potential downtime/deprecation/big changes, an entry has been made in Updating DataHub

There 2 big improvements:

ClickHouseConfig inherits TwoTierSQLAlchemyConfig, ClickHouseSource inherit TwoTierSQLAlchemySource
Added uri_opt params, old uri_opts cannot be changed or extended via config.

…roject#6544

…#6544

…move_to_TwoTierSqlalchemy

mayurinehate

Could you please also update integration tests / golden files with respect to the changes ?

metadata-ingestion/src/datahub/ingestion/source/sql/clickhouse.py

metadata-ingestion/docs/sources/clickhouse/clickhouse_recipe.yml

metadata-ingestion/src/datahub/ingestion/source/sql/clickhouse.py

…qlalchemy

metadata-ingestion/src/datahub/ingestion/source/sql/clickhouse.py

metadata-ingestion/docs/sources/clickhouse/clickhouse_recipe.yml

metadata-ingestion/src/datahub/ingestion/source/sql/clickhouse.py

…qlalchemy # Conflicts: # docs/how/updating-datahub.md

…qlalchemy

added source Co-authored-by: Mayuri Nehate <33225191+mayurinehate@users.noreply.github.com>

metadata-ingestion/src/datahub/ingestion/source/sql/clickhouse.py

…house)_move_to_TwoTierSqlalchemy

…house)_move_to_TwoTierSqlalchemy # Conflicts: # docs/how/updating-datahub.md

docs/how/updating-datahub.md

mayurinehate · 2023-08-07T08:56:13Z

metadata-ingestion/tests/integration/clickhouse/test_clickhouse.py

+
+
+@freeze_time(FROZEN_TIME)
+@pytest.mark.integration


Thank for adding the test. This is not the test I was looking for. I was looking for unit test similar to test_clickhouse_uri_native_secure in test_clickhouse_source.py for clickhouse secure and protocol config that demonstrate backward compatibility if these configs are used.

The current test looks good too, however, I believe the golden files are exactly same as existing clickhouse_mces_golden.json, except the run_id. I'd suggest using the same golden files in check_golden_file by using same run_id in new sqlalchemy uri recipe, rather than adding a duplicate 99% same golden file.

…house)_move_to_TwoTierSqlalchemy

mayurinehate

lgtm.

metadata-ingestion/tests/unit/test_clickhouse_source.py

Co-authored-by: Mayuri Nehate <33225191+mayurinehate@users.noreply.github.com>

asikowitz · 2023-08-11T20:11:37Z

Merging as should not be related to smoke test failure

Co-authored-by: Mayuri Nehate <33225191+mayurinehate@users.noreply.github.com>

alplatonov added 5 commits June 23, 2023 18:48

fix(ingest/clickhouse)_Ingest Clickhouse show None database datahub-p…

d54f569

…roject#6544

fix(ingest/clickhouse)_Ingest Clickhouse show None database datahub-p…

3f41f37

…roject#6544

fix(ingest/clickhouse)_update docs and removed unused check on secure…

4e68f23

…#6544

fix(ingest/clickhouse)_update docs and removed unused check on secure…

048e04e

…#6544

fix(ingest/clickhouse)_update docs and removed unused check on secure…

26e16b7

…#6544

github-actions bot added docs Issues and Improvements to docs ingestion PR or Issue related to the ingestion of metadata labels Jun 26, 2023

alplatonov mentioned this pull request Jun 26, 2023

Ingest Clickhouse show None database #6544

Closed

vercel bot deployed to Preview June 26, 2023 10:01 View deployment

Merge branch 'datahub-project:master' into fix(ingestion/clickhouse)_…

8cccbaf

…move_to_TwoTierSqlalchemy

vercel bot deployed to Preview June 26, 2023 10:45 View deployment

hsheth2 requested a review from mayurinehate June 26, 2023 18:59

mayurinehate suggested changes Jun 27, 2023

View reviewed changes

anshbansal added the community-contribution PR or Issue raised by member(s) of DataHub Community label Jun 28, 2023

fix(ingest/clickhouse)_fix_review_issues

0455f32

vercel bot deployed to Preview June 28, 2023 13:26 View deployment

Merge branch 'master' into fix(ingestion/clickhouse)_move_to_TwoTierS…

e09a365

…qlalchemy

vercel bot deployed to Preview June 28, 2023 14:38 View deployment

fix(ingest/clickhouse)_revrite_make_url_import and opts

8baa662

vercel bot deployed to Preview June 28, 2023 16:33 View deployment

alplatonov added 2 commits June 28, 2023 20:44

Merge branch 'master' into fix(ingestion/clickhouse)_move_to_TwoTierS…

d2ac84f

…qlalchemy

fix(ingest/clickhouse)_fix flake8 errors

a4c2edd

vercel bot deployed to Preview June 28, 2023 17:24 View deployment

mayurinehate reviewed Jul 3, 2023

View reviewed changes

Merge branch 'master' into fix(ingestion/clickhouse)_move_to_TwoTierS…

71ec2c8

…qlalchemy # Conflicts: # docs/how/updating-datahub.md

vercel bot deployed to Preview July 14, 2023 12:18 View deployment

maggiehays added the pending-submitter-response Issue/request has been reviewed but requires a response from the submitter label Jul 17, 2023

alplatonov and others added 3 commits July 20, 2023 18:44

Merge branch 'master' into fix(ingestion/clickhouse)_move_to_TwoTierS…

a83bd4b

…qlalchemy

fix(ingest/clickhouse)_fix import

1efb272

Update metadata-ingestion/docs/sources/clickhouse/clickhouse_recipe.yml

abf51c8

added source Co-authored-by: Mayuri Nehate <33225191+mayurinehate@users.noreply.github.com>

fix(ingest/clickhouse)_fix mypy errors

f20d3b9

vercel bot deployed to Preview July 24, 2023 20:06 View deployment

fix(ingest/clickhouse)_fix tests

0ada1f1

vercel bot deployed to Preview July 25, 2023 07:17 View deployment

Merged master & updated golden file

53577bc

vercel bot deployed to Preview July 25, 2023 08:17 View deployment

fix(ingest/clickhouse)_fix black errors

9365d0d

vercel bot deployed to Preview July 25, 2023 08:47 View deployment

mayurinehate reviewed Jul 25, 2023

View reviewed changes

metadata-ingestion/src/datahub/ingestion/source/sql/clickhouse.py Outdated Show resolved Hide resolved

mayurinehate reviewed Jul 31, 2023

View reviewed changes

metadata-ingestion/src/datahub/ingestion/source/sql/clickhouse.py Outdated Show resolved Hide resolved

alplatonov added 4 commits July 31, 2023 15:57

Merge remote-tracking branch 'origin/master' into fix(ingestion/click…

e76ad11

…house)_move_to_TwoTierSqlalchemy

fix(ingest/ldap)_added_comments + new test for sqlalchemi_uri form

1d04e9e

Merge remote-tracking branch 'origin/master' into fix(ingestion/click…

9d5c583

…house)_move_to_TwoTierSqlalchemy # Conflicts: # docs/how/updating-datahub.md

fix(ingest/ldap)_added docs for ingestion

d6d6fb2

vercel bot deployed to Preview August 4, 2023 11:19 View deployment

mayurinehate reviewed Aug 7, 2023

View reviewed changes

alplatonov added 2 commits August 7, 2023 14:19

Merge remote-tracking branch 'origin/master' into fix(ingestion/click…

35bea4e

…house)_move_to_TwoTierSqlalchemy

fix(ingest/ldap)_fix docs & test

d3d5fde

vercel bot deployed to Preview August 7, 2023 10:41 View deployment

alplatonov added 2 commits August 10, 2023 21:40

Merge remote-tracking branch 'origin/master' into fix(ingestion/click…

f546561

…house)_move_to_TwoTierSqlalchemy

fix(ingest/ldap)_added unittest for depricated attributes

1c09763

vercel bot deployed to Preview August 10, 2023 18:23 View deployment

mayurinehate approved these changes Aug 11, 2023

View reviewed changes

metadata-ingestion/tests/unit/test_clickhouse_source.py Outdated Show resolved Hide resolved

metadata-ingestion/tests/unit/test_clickhouse_source.py Outdated Show resolved Hide resolved

alplatonov and others added 2 commits August 11, 2023 09:40

Update metadata-ingestion/tests/unit/test_clickhouse_source.py

3106cad

Co-authored-by: Mayuri Nehate <33225191+mayurinehate@users.noreply.github.com>

Update metadata-ingestion/tests/unit/test_clickhouse_source.py

a8e0331

Co-authored-by: Mayuri Nehate <33225191+mayurinehate@users.noreply.github.com>

vercel bot deployed to Preview August 11, 2023 06:11 View deployment

asikowitz approved these changes Aug 11, 2023

View reviewed changes

asikowitz merged commit 11fdfcf into datahub-project:master Aug 11, 2023
43 of 44 checks passed

yoonhyejin pushed a commit that referenced this pull request Aug 24, 2023

Fix(ingestion/clickhouse) move to two tier sqlalchemy (#8300)

6487e5c

Co-authored-by: Mayuri Nehate <33225191+mayurinehate@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix(ingestion/clickhouse) move to two tier sqlalchemy #8300

Fix(ingestion/clickhouse) move to two tier sqlalchemy #8300

alplatonov commented Jun 26, 2023

mayurinehate left a comment

mayurinehate Aug 7, 2023

mayurinehate left a comment

asikowitz commented Aug 11, 2023



		@freeze_time(FROZEN_TIME)
		@pytest.mark.integration

Fix(ingestion/clickhouse) move to two tier sqlalchemy #8300

Fix(ingestion/clickhouse) move to two tier sqlalchemy #8300

Conversation

alplatonov commented Jun 26, 2023

Checklist

mayurinehate left a comment

Choose a reason for hiding this comment

mayurinehate Aug 7, 2023

Choose a reason for hiding this comment

mayurinehate left a comment

Choose a reason for hiding this comment

asikowitz commented Aug 11, 2023