Skip to content

Conversation

@m-abulazm
Copy link
Contributor

@m-abulazm m-abulazm commented Nov 20, 2025

Changes

What does this PR do?

Add prompts during configure-reconcile for the data sources' credentials. this allows users to use local, env or databricks secrets.
this is more flexible and secure than the current approach that requires users to create a secret specifically for reconcile and now users can reuse their existing secrets.

Also allows reconcile to support new vault types e.g. azure, google secrets as we implement more supported backends in the future

Linked issues

Progresses #1008, #2123, #2159

Functionality

  • added relevant user documentation
  • modified existing command: databricks labs lakebridge configure-reconcile

Tests

  • manually tested
  • added unit tests
  • added integration tests

@m-abulazm m-abulazm self-assigned this Nov 20, 2025
@m-abulazm m-abulazm requested a review from a team as a code owner November 20, 2025 15:36
@m-abulazm m-abulazm added feat/cli actions that are visible to the user feat/recon making sure that remorphed query produces the same results as original stacked PR Should be reviewed, but not merged labels Nov 20, 2025
@codecov
Copy link

codecov bot commented Nov 20, 2025

Codecov Report

❌ Patch coverage is 94.11765% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.07%. Comparing base (cf4fe77) to head (44ea015).

Files with missing lines Patch % Lines
...icks/labs/lakebridge/helpers/recon_config_utils.py 93.47% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@                    Coverage Diff                     @@
##           refactor/creds-manager    #2157      +/-   ##
==========================================================
- Coverage                   64.30%   64.07%   -0.24%     
==========================================================
  Files                         100      100              
  Lines                        8747     8696      -51     
  Branches                      918      915       -3     
==========================================================
- Hits                         5625     5572      -53     
- Misses                       2947     2948       +1     
- Partials                      175      176       +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
Copy link

github-actions bot commented Nov 20, 2025

✅ 51/51 passed, 6 flaky, 4m14s total

Flaky tests:

  • 🤪 test_transpiles_informatica_to_sparksql_non_interactive[True] (22.099s)
  • 🤪 test_transpiles_informatica_to_sparksql (24.265s)
  • 🤪 test_transpile_teradata_sql_non_interactive[True] (22.718s)
  • 🤪 test_transpiles_informatica_to_sparksql_non_interactive[False] (4.232s)
  • 🤪 test_transpile_teradata_sql_non_interactive[False] (22.287s)
  • 🤪 test_transpile_teradata_sql (6.822s)

Running from acceptance #3275

@m-abulazm m-abulazm added the documentation Improvements or additions to documentation label Nov 21, 2025
# Conflicts:
#	src/databricks/labs/lakebridge/install.py
#	tests/unit/test_install.py
```
If not set the default values will be used to store the metadata. The default resources are created during the installation
of Lakebridge.
- `creds_or_secret_scope`: The credentials to use to connect to the data source. Made optional for backwards compatibility.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to give this flexibility ?
I would rather force them to use secret scope for reconcile do not open up.

if this is required for testing then we can explore other options.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed back to secret scope

# Conflicts:
#	src/databricks/labs/lakebridge/install.py
#	src/databricks/labs/lakebridge/reconcile/trigger_recon_service.py
#	tests/integration/reconcile/query_builder/test_execute.py
#	tests/integration/reconcile/test_oracle_reconcile.py
#	tests/unit/deployment/test_installation.py
#	tests/unit/deployment/test_job.py
#	tests/unit/deployment/test_recon.py
#	tests/unit/test_install.py
m-abulazm added a commit that referenced this pull request Dec 22, 2025
…#2159)

<!-- REMOVE IRRELEVANT COMMENTS BEFORE CREATING A PULL REQUEST -->
## Changes
<!-- Summary of your changes that are easy to understand. Add
screenshots when necessary, they're helpful to illustrate the before and
after state -->
### What does this PR do?
* Move away from hardcoded secrets in reconcile
* use credential manager which enables local, env and databricks

### Relevant implementation details
* add `load_credentials` to `DataSource` which takes care of loading the
credentials

### Caveats/things to watch out for when reviewing:

### Linked issues
<!-- DOC: Link issue with a keyword: close, closes, closed, fix, fixes,
fixed, resolve, resolves, resolved. See
https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
-->

Progresses #1008,
#2123,
#2157

### Functionality

- [ ] added relevant user documentation
- [ ] added new CLI command
- [X] modified existing command: `databricks labs lakebridge reconcile`
- [ ] ... +add your own

### Tests
<!-- How is this tested? Please see the checklist below and also
describe any other relevant tests -->

- [ ] manually tested
- [X] added unit tests
- [X] added integration tests

---------

Co-authored-by: Guenia Izquierdo <[email protected]>
# Conflicts:
#	src/databricks/labs/lakebridge/install.py
#	tests/unit/test_install.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation feat/cli actions that are visible to the user feat/recon making sure that remorphed query produces the same results as original stacked PR Should be reviewed, but not merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants