-
Notifications
You must be signed in to change notification settings - Fork 72
Introduce a new feature to establish base configurator for Profiler Assessment #1981
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.
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
base: main
Are you sure you want to change the base?
Conversation
✅ 29/29 passed, 2 flaky, 1m23s total Flaky tests:
Running from acceptance #2397 |
Merged #1940 in this co-authored by: @goodwillpunning |
logger.info(f"Creating a virtual environment for Python script execution: ${venv_dir}") | ||
# Install dependencies in the virtual environment | ||
# Define the paths to the virtual environment's Python and pip executables | ||
if sys.platform == "win32": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to standardize this with databricks.labs.lakebridge.install.WheelInstaller._create_venv
please add a TODO for later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- create_env
- activate env
- _install_dependencies
- _run_python_script
are all generic and can be extracted out to a common utility class
"snowflake": SnowflakeConnector, | ||
"mssql": MSSQLConnector, | ||
"tsql": MSSQLConnector, | ||
"synapse": MSSQLConnector, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Changes
What does this PR do?
Relevant implementation details
databricks labs lakebridge configure-database-profiler
Caveats/things to watch out for when reviewing:
Linked issues
Resolves #..
Functionality
databricks labs lakebridge ...
Tests