Skip to content

improve test support of test mssql instance #1937

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

improve test support of test mssql instance #1937

wants to merge 7 commits into from
+81 −127

Conversation

m-abulazm
Copy link
Contributor

@m-abulazm m-abulazm commented Aug 14, 2025 •
edited
Loading

Changes

What does this PR do?

  1. Makes integrations tests with sqlserver more straightforward and easier to develop by removing a patch that was needed
  2. extends DatabaseManager with commit behavior if needed otherwise all database write operations are rolled back

Functionality

  • added relevant user documentation
  • added new CLI command
  • modified existing command: databricks labs lakebridge ...

Tests

  • manually tested
  • added unit tests
  • added integration tests

@m-abulazm m-abulazm requested a review from a team as a code owner August 14, 2025 14:55
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 14, 2025 •
edited
Loading

✅ 27/27 passed, 1 flaky, 1m23s total

Flaky tests:

  • 🤪 test_transpiles_informatica_with_sparksql (9.668s)

Running from acceptance #2325

@m-abulazm m-abulazm force-pushed the refactor/tests/remove-creds-patch branch from be0e24b to ed83d90 Compare August 14, 2025 15:07
@m-abulazm m-abulazm requested a review from asnare August 18, 2025 13:17
@m-abulazm m-abulazm self-assigned this Aug 20, 2025
sundarshankar89
sundarshankar89 approved these changes Sep 9, 2025
View reviewed changes
Copy link
Collaborator

@sundarshankar89 sundarshankar89 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM small nit:

sundarshankar89
sundarshankar89 requested changes Sep 9, 2025
View reviewed changes
@m-abulazm m-abulazm added tech debt design flaws and other cascading effects internal technical pr's not end user facing labels Sep 9, 2025
@m-abulazm m-abulazm force-pushed the refactor/tests/remove-creds-patch branch from bd3deb3 to 88e8702 Compare September 10, 2025 08:52
sundarshankar89
sundarshankar89 approved these changes Sep 18, 2025
View reviewed changes
Copy link
Collaborator

@sundarshankar89 sundarshankar89 left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sundarshankar89 sundarshankar89 sundarshankar89 approved these changes

@gueniai gueniai Awaiting requested review from gueniai

@asnare asnare Awaiting requested review from asnare

At least 2 approving reviews are required to merge this pull request.

Assignees

@m-abulazm m-abulazm

Labels
internal technical pr's not end user facing tech debt design flaws and other cascading effects
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@m-abulazm @sundarshankar89 @gueniai