remove redundant reconcile config params #2150

m-abulazm · 2025-11-17T15:30:51Z

Changes

What does this PR do?

Remove redundant reconcile config

Resolves #518

Functionality

added relevant user documentation
added new CLI command
modified existing command: databricks labs lakebridge ...
... +add your own

Tests

manually tested
added unit tests
added integration tests

codecov · 2025-11-17T15:34:30Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.01%. Comparing base (eebf284) to head (53d37ab).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2150      +/-   ##
==========================================
- Coverage   65.24%   65.01%   -0.24%     
==========================================
  Files         100      100              
  Lines        8506     8464      -42     
  Branches      876      876              
==========================================
- Hits         5550     5503      -47     
- Misses       2769     2773       +4     
- Partials      187      188       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2025-11-17T15:40:03Z

✅ 51/51 passed, 10 flaky, 3m26s total

Flaky tests:

🤪 test_validate_non_empty_tables (19ms)
🤪 test_validate_mixed_checks (169ms)
🤪 test_validate_invalid_schema_path (0s)
🤪 test_validate_successful_schema_check (289ms)
🤪 test_validate_invalid_schema_check (1ms)
🤪 test_transpiles_informatica_to_sparksql (20.511s)
🤪 test_transpile_teradata_sql (24.356s)
🤪 test_transpiles_informatica_to_sparksql_non_interactive[True] (3.827s)
🤪 test_transpile_teradata_sql_non_interactive[False] (5.474s)
🤪 test_transpile_teradata_sql_non_interactive[True] (5.509s)

_{Running from acceptance #3072}

gueniai

LGTM

asnare

We're modifying a versioned configuration file that we store in the user's workspace, so we really need to deal with compatibility and ensure there is test coverage for compatibility.

asnare · 2025-12-01T13:45:14Z

docs/lakebridge/static/lakebridge_reconcile/lakebridge_recon_main.html

I'm not sure what the changes in here are, this looks like static documentation setup rather than content-related?

this is reconcile notebook exported as html from databricks workspace. the change in the notebook is deleting the removed config

asnare · 2025-12-01T13:49:03Z

src/databricks/labs/lakebridge/config.py

-    source_schema: str
-    target_catalog: str
-    target_schema: str
    tables: list[Table]
-    source_catalog: str | None = None


If we're changing the schema of something we store in the user's workspace, I think we need:

To bump the version number.

Tests for the migration/loading path.

Even if loading ignores (now unknown) fields, an older version of the code will choke if it tries to load an instance persisted with this new schema.

since this is a backwards compatible change, I didn't bump the version. So I added a test for testing that.

, an older version of the code will choke if it tries to load an instance persisted with this new schema.

And I am not aware of any forward-compatibility support in blueprint. or did I misunderstood something?

Here's the issue:

an older version of the code will choke if it tries to load an instance persisted with this new schema.

The scenario is:

Someone runs with this version of the code.

The file is created.

For whatever reason (downgrade, cut'n'paste to another team-member, different environment), someone runs an older version of the code.

This leads to:

If the version is not bumped: the older version will choke because it expects fields that are missing.

If the version is bumped: the older version will detect that it can't be loaded and raise an IllegalState error with something like "expected version 1, got 2".

Both are errors, but the second one is controlled and expected, and also much easier to diagnose.

got it. just pushed version 2

asnare · 2025-12-01T13:49:47Z

src/databricks/labs/lakebridge/config.py

-    job_id: str | None = None
-    tables: ReconcileTablesConfig | None = None


Also here, regarding version/schema change.

we can bump the version but I would not (here at least).

this was never set in the old code and has no impact on the existing installations

Implement databricks creds manager #2123 bumps the version so no need to do it wice

these are optional values so the older code will continue working

sundarshankar89

LGTM

remove redundant config params

773bad2

m-abulazm self-assigned this Nov 17, 2025

m-abulazm requested a review from a team as a code owner November 17, 2025 15:30

m-abulazm added documentation Improvements or additions to documentation feat/recon making sure that remorphed query produces the same results as original labels Nov 17, 2025

m-abulazm requested review from gueniai and sundarshankar89 November 17, 2025 15:31

m-abulazm temporarily deployed to tool November 17, 2025 15:32 — with GitHub Actions Inactive

Merge branch 'main' into refactor/518-remove-redundant

5ccce14

m-abulazm temporarily deployed to tool November 17, 2025 15:49 — with GitHub Actions Inactive

m-abulazm changed the title ~~remove redundant config params~~ remove redundant reconcile config params Nov 17, 2025

m-abulazm enabled auto-merge November 17, 2025 19:53

gueniai approved these changes Nov 26, 2025

View reviewed changes

Merge branch 'main' into refactor/518-remove-redundant

864322f

m-abulazm temporarily deployed to tool November 27, 2025 08:49 — with GitHub Actions Inactive

asnare requested changes Dec 1, 2025

View reviewed changes

m-abulazm added 2 commits December 2, 2025 12:51

remove unused code and duplicate tests

3dcb896

add test

619a3de

m-abulazm temporarily deployed to tool December 2, 2025 11:53 — with GitHub Actions Inactive

m-abulazm requested a review from asnare December 2, 2025 11:59

Merge branch 'main' into refactor/518-remove-redundant

7fc114f

m-abulazm temporarily deployed to tool December 2, 2025 11:59 — with GitHub Actions Inactive

bump TableRecon version

f661fd1

m-abulazm temporarily deployed to tool December 2, 2025 12:47 — with GitHub Actions Inactive

fmt

53d37ab

m-abulazm temporarily deployed to tool December 2, 2025 20:39 — with GitHub Actions Inactive

sundarshankar89 approved these changes Dec 5, 2025

View reviewed changes

		job_id: str \| None = None
		tables: ReconcileTablesConfig \| None = None

remove redundant reconcile config params #2150

Are you sure you want to change the base?

remove redundant reconcile config params #2150

Conversation

m-abulazm commented Nov 17, 2025

Changes

What does this PR do?

Functionality

Tests

Uh oh!

codecov bot commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gueniai left a comment

Choose a reason for hiding this comment

Uh oh!

asnare left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sundarshankar89 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Nov 17, 2025 •

edited

Loading

github-actions bot commented Nov 17, 2025 •

edited

Loading