Redshift Profiler PR1: Redshift connection and credential support in database_manager

[ysmx-github]

Changes

This PR adds Redshift as a supported profiler assessment platform.

What does this PR do?

• Adds Redshift as a supported platform for the profiler assessment (alongside Synapse).
• Implements Redshift credential flows: database password, federated user, secrets manager (ARN), temporary credentials db user, temporary credentials IAM; allows SSL for all; configurator prompts and connection handling in database_manager.py.
• Files changed
  • .gitignore
  • pyproject.toml
  • src/databricks/labs/lakebridge/connections/database_manager.py

Relevant implementation details

• Credentials: database password, federated user, secrets manager (ARN), temporary credentials db user, temporary credentials IAM. Federated and Secrets Manager use an optional lazy boto3 import.

Caveats/things to watch out for when reviewing:

Linked issues

Resolves #..

Functionality

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

Tests

1. Manually Tested all credential flows on all clusters in AWS Sandbox account aws-sandbox-field-eng (332745928618)
   tests/resources/assessments/pipeline_config_main_redshift.yml: pipeline config that runs that script.

• manually tested
• added unit tests
• added integration tests

[codecov]

Codecov Report

❌ Patch coverage is 36.00000% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.23%. Comparing base (41c3f9a) to head (ce0e10e).

Files with missing lines	Patch %	Lines
...ks/labs/lakebridge/connections/database_manager.py	36.00%	32 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2305      +/-   ##
==========================================
- Coverage   66.41%   66.23%   -0.19%     
==========================================
  Files          99       99              
  Lines        9094     9130      +36     
  Branches      974      981       +7     
==========================================
+ Hits         6040     6047       +7     
- Misses       2878     2907      +29     
  Partials      176      176              

☔ 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]

✅ 145/145 passed, 8 flaky, 4 skipped, 41m57s total

Flaky tests:

• 🤪 test_installs_and_runs_local_bladebridge (20.765s)
• 🤪 test_installs_and_runs_pypi_bladebridge (28.313s)
• 🤪 test_transpiles_informatica_to_sparksql_non_interactive[False] (18.114s)
• 🤪 test_transpiles_informatica_to_sparksql_non_interactive[True] (18.531s)
• 🤪 test_transpiles_informatica_to_sparksql (20.282s)
• 🤪 test_transpile_teradata_sql (19.69s)
• 🤪 test_transpile_teradata_sql_non_interactive[True] (5.974s)
• 🤪 test_transpile_teradata_sql_non_interactive[False] (5.986s)

_{Running from acceptance #4001}

[sundarshankar89]

small nits: but overall good to go.
main thing is https://github.com/databrickslabs/lakebridge/blob/main/docs/lakebridge/docs/dev/contributing.md#gpg-signing all commits need to signed can you fix that by following the steps and rewriting the commit history in git for this PR.

@asnare can you take a look,
What about https://docs.localstack.cloud/aws/services/redshift/ for testing?

[sundarshankar89]

this is not notebook.

[sundarshankar89]

Suggested change

	exclude = ["tests/resources/.*", "src/databricks/labs/lakebridge/resources/assessments/.*"]
	exclude = ["tests/resources/.*"]

[sundarshankar89]

This is needed fo synapse assessment.

[sundarshankar89]

do we still need this?

[sundarshankar89]

even this should match main.

[asnare]

Aside from a few nits, some things I'm missing:

• For new dependencies, we need a note on the licensing terms. Often this is straightforward but for anything AWS-related I'd like to see it.
• It's unclear to me why this new source doesn't use SQLAlchemy like the others? That's made the implementation more complex than otherwise, with accompanying duplicate code because it doesn't extend _BaseConnector. What makes Redshift special and different to the other sources we support?
• We absolutely need some test coverage: we don't have the resources to manually test this, and without automated coverage of some sort the chances of this being inadvertently broken are high. I understand that for this integration testing is going to be difficult to set up, but at the very least there needs to be some unit coverage.

[asnare]

This looks suspicious to me: why does the interface have to cater specifically for RedshiftConnection? Shouldn't RedshiftConnection conform to the existing contract?

[sundarshankar89]

this is because there used to be bridge lib

sqlalchemy/sqlalchemy#11950 which is abandon.
This is compromise given the connector returns alchmey compliant api for fetch. which is self contained within the database_manager.py, but happy to hear alternatives.

[asnare]

For a given module it's normally best to either use one or the other…

Suggested change

	import redshift_connector # type: ignore[import-untyped]
	from redshift_connector import Connection as RedshiftConnection # type: ignore[import-untyped]
	import redshift_connector # type: ignore[import-untyped]

[asnare]

• We absolutely need some test coverage: we don't have the resources to manually test this, and without automated coverage of some sort the chances of this being inadvertently broken are high. I understand that for this integration testing is going to be difficult to set up, but at the very least there needs to be some unit coverage.

To clarify a bit here:

• We absolutely need integration tests against the real thing where possible: that's the source of truth, after all. At a technical level local emulations often don't behave the same way, and the difference leads to surprises.
• There's nothing wrong with a local version of the service, especially for use during development. In fact this is preferable: you want the dev-loop to be local and fast, avoiding round-tripping through CI/CD is a massive win.
• The local version here is a nice-to-have, and doesn't supersede the need for proper integration testing against the real thing. We're also going to need the real thing for support and troubleshooting.

Getting back to your original question about LocalStack:

• This is a 3rd-party emulation, so I expect there will be significant drift relative to the real thing. (This is consistent with what I wrote above: we can't rely on it for testing as a source of truth.)
• It's a commercial offering, with tiers. I can't make sense of it all, but it looks to me like: a) per-seat licensing; b) CI/CD uses a credit-based system. (It's unclear to me if the 'Pro' image is licensed separately, but I would guess we need the 'Pro' image because profiling would use some of the APIs that it provided.)