-
Notifications
You must be signed in to change notification settings - Fork 78
Add Switch transpiler with --include-llm-transpiler flag
#2066
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
- Add Switch installer with resource configuration and job creation - Implement uninstall functionality with proper cleanup - Add comprehensive test coverage for SwitchInstaller - Improve path handling and type-safe configuration - Add include-llm-transpiler option for flexible installation
|
✅ 51/51 passed, 9 flaky, 3m48s total Flaky tests:
Running from acceptance #2883 |
Implement SwitchInstaller to integrate Switch transpiler with Lakebridge: - Install Switch package to local virtual environment and deploy to workspace - Create and manage Databricks job for Switch transpilation - Configure Switch resources (catalog, schema, volume) interactively - Support job-level parameters with JobParameterDefinition for flexibility - Handle installation state and job lifecycle management - Add comprehensive test suite covering installation, job management, and configuration
0f749b8 to
7cb9ea9
Compare
The SwitchInstaller was failing to find the config when the config.yml used "Switch" (capitalized) as the name, while the code only checked for "switch" (lowercase). This caused job creation to fail with a "config.yml not found" error. Updated _get_switch_job_parameters() to check both the display name (capitalized) and transpiler ID (lowercase) to handle both cases.
| wheel_name = self._PYPI_PACKAGE_NAME.replace("-", "_") | ||
| return wheel_name in artifact.name and artifact.suffix == ".whl" | ||
|
|
||
| def install(self, artifact: Path | None = None) -> bool: |
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.
This is how we have separated things out: the installer installs things locally, deployer deploys to the workspace. You need something similar to recon deployer that way you don't need to have workspace client inside TranspilerInstaller
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.
Thanks for the feedback. Updated in 8439314 - created SwitchDeployment for workspace operations (following ReconDeployment pattern) and removed workspace dependencies from TranspilerInstaller.
Separates Switch transpiler's local installation logic from workspace deployment, following established patterns (BladebridgeInstaller for local installation, ReconDeployment for workspace deployment). Key changes: - Add SwitchDeployment class (~260 lines) for workspace operations - Simplify SwitchInstaller to match BladebridgeInstaller pattern (~20 lines) - Add include_llm and switch_resources fields to TranspileConfig - Update WorkspaceInstallation to use SwitchDeployment - Refactor tests to avoid protected member access using fixture separation - Group Switch-related tests in TestSwitchInstallation class
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
Less noise during normal operation, more information with --debug.
| except (RuntimeError, ValueError, InvalidParameterValue) as e: | ||
| logger.error(f"Failed to create/update Switch job: {e}") |
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.
This path will lead to confusion, because we return normally. Subsequent logs include:
DEBUG: Switch deployment completed.INFO: Installation completed successfully! Please refer to the documentation for the next steps.
Neither are true. The easiest fix right now is to return a boolean indicating success. (Alternatively, we could re-raise the exception.)
Note: we also currently lose the stacktrace on failure. Maybe logger.exception() instead.
Changes
This PR adds Switch, an LLM-powered transpiler, as an optional component in Lakebridge's
install-transpileworkflow. Switch installation is controlled by the new--include-llm-transpilerflag.What does this PR do?
Implements complete Switch transpiler integration with idempotent installation, workspace deployment, job management, and resource configuration including Unity Catalog Volume for Switch.
Relevant implementation details
CLI Integration:
--include-llm-transpilerflag toinstall-transpilecommand (default:false)SwitchInstaller Implementation:
TranspilerInstallerwith unified constructor signaturedatabricks-switch-plugin)/Users/{user}/.lakebridge/switch/LAKEBRIDGE_Switchjob with NotebookTask for parallel LLM processingUninstall Integration:
databricks labs uninstall lakebridgeworkflowCaveats/things to watch out for when reviewing:
--include-llm-transpilerflag to install Switchinclude-llm-transpiler, "true"to user agentmax-argsfrom 12 to 13 inpyproject.tomlto accommodate new CLI parameterLinked issues
Resolves #2048
Console Output
Functionality
databricks labs lakebridge install-transpile(adds--include-llm-transpilerflag and Switch installation support)databricks labs uninstall lakebridge(adds Switch job and resource cleanup)Tests