Skip to content

Commit d441ad8

Browse files
JCZuurmondnfx
andauthored
Fix create database in workspace installation (#2224)
## Changes Database creation should be done asynchronously. Mypy probably missed this as the method did not have any type hints. Introduced in #1920 --------- Co-authored-by: Serge Smertin <[email protected]>
1 parent a3db6ce commit d441ad8

File tree

3 files changed

+22
-15
lines changed

3 files changed

+22
-15
lines changed

src/databricks/labs/ucx/install.py

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -490,12 +490,17 @@ def config(self):
490490
def folder(self):
491491
return self._installation.install_folder()
492492

493-
def run(self):
493+
def run(self) -> bool:
494+
"""Run workflow installation.
495+
496+
Returns
497+
bool :
498+
True, installation finished. False installation did not finish.
499+
"""
494500
logger.info(f"Installing UCX v{self._product_info.version()}")
495-
install_tasks = [self._create_database()] # Need the database before creating the dashboards
496-
install_tasks.extend(self._create_dashboards())
501+
install_tasks = [self._create_database_and_dashboards, self._workflows_installer.create_jobs]
497502
Threads.strict("installing components", install_tasks)
498-
readme_url = self._workflows_installer.create_jobs()
503+
readme_url = self._installation.workspace_link("README")
499504
if not self._is_account_install and self._prompts.confirm(f"Open job overview in your browser? {readme_url}"):
500505
webbrowser.open(readme_url)
501506
logger.info(f"Installation completed successfully! Please refer to the {readme_url} for the next steps.")
@@ -504,6 +509,10 @@ def run(self):
504509
self._trigger_workflow("assessment")
505510
return True
506511

512+
def _create_database_and_dashboards(self) -> None:
513+
self._create_database() # Need the database before creating the dashboards
514+
Threads.strict("installing dashboards", list(self._get_create_dashboard_tasks()))
515+
507516
def _create_database(self):
508517
try:
509518
deploy_schema(self._sql_backend, self._config.inventory_database)
@@ -519,8 +528,8 @@ def _create_database(self):
519528
raise BadRequest(msg) from err
520529
raise err
521530

522-
def _create_dashboards(self) -> Iterable[Callable[[], None]]:
523-
"""Create the lakeview dashboards from the SQL queries in the queries subfolders"""
531+
def _get_create_dashboard_tasks(self) -> Iterable[Callable[[], None]]:
532+
"""Get the tasks to create Lakeview dashboards from the SQL queries in the queries subfolders"""
524533
logger.info("Creating dashboards...")
525534
dashboard_folder_remote = f"{self._installation.install_folder()}/dashboards"
526535
try:
@@ -544,7 +553,7 @@ def _create_dashboards(self) -> Iterable[Callable[[], None]]:
544553

545554
# TODO: Confirm the assumption below is correct
546555
# An InternalError may occur when the dashboard is being published and the database does not exists
547-
@retried(on=[InternalError], timeout=timedelta(minutes=2))
556+
@retried(on=[InternalError], timeout=timedelta(minutes=4))
548557
def _create_dashboard(self, folder: Path, *, parent_path: str | None = None) -> None:
549558
"""Create a lakeview dashboard from the SQL queries in the folder"""
550559
logger.info(f"Creating dashboard in {folder}...")

src/databricks/labs/ucx/installer/workflows.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -417,11 +417,11 @@ def __init__(
417417
self._this_file = Path(__file__)
418418
super().__init__(config, installation, ws)
419419

420-
def create_jobs(self):
420+
def create_jobs(self) -> None:
421421
remote_wheels = self._upload_wheel()
422422
desired_workflows = {t.workflow for t in self._tasks if t.cloud_compatible(self._ws.config)}
423-
wheel_runner = None
424423

424+
wheel_runner = ""
425425
if self._config.override_clusters:
426426
wheel_runner = self._upload_wheel_runner(remote_wheels)
427427
for workflow_name in desired_workflows:
@@ -446,7 +446,7 @@ def create_jobs(self):
446446

447447
self._install_state.save()
448448
self._create_debug(remote_wheels)
449-
return self._create_readme()
449+
self._create_readme()
450450

451451
@property
452452
def _config_file(self):
@@ -464,7 +464,7 @@ def _get_test_purge_time(self) -> str:
464464
timeout = TEST_NIGHTLY_CI_JOBS_PURGE_TIMEOUT if self._is_nightly() else TEST_JOBS_PURGE_TIMEOUT
465465
return (datetime.utcnow() + timeout).strftime("%Y%m%d%H")
466466

467-
def _create_readme(self) -> str:
467+
def _create_readme(self) -> None:
468468
debug_notebook_link = self._installation.workspace_markdown_link('debug notebook', 'DEBUG.py')
469469
markdown = [
470470
"# UCX - The Unity Catalog Migration Assistant",
@@ -496,8 +496,6 @@ def _create_readme(self) -> str:
496496
preamble = ["# Databricks notebook source", "# MAGIC %md"]
497497
intro = "\n".join(preamble + [f"# MAGIC {line}" for line in markdown])
498498
self._installation.upload('README.py', intro.encode('utf8'))
499-
readme_url = self._installation.workspace_link('README')
500-
return readme_url
501499

502500
def _create_dashboard_links(self, step_name, dashboard_link):
503501
dashboards_per_step = [d for d in self._install_state.dashboards.keys() if d.startswith(step_name)]
@@ -570,7 +568,7 @@ def _upload_wheel(self):
570568
wheel_paths = [f"/Workspace{wheel}" for wheel in wheel_paths]
571569
return wheel_paths
572570

573-
def _upload_wheel_runner(self, remote_wheels: list[str]):
571+
def _upload_wheel_runner(self, remote_wheels: list[str]) -> str:
574572
# TODO: we have to be doing this workaround until ES-897453 is solved in the platform
575573
remote_wheels_str = " ".join(remote_wheels)
576574
code = TEST_RUNNER_NOTEBOOK.format(remote_wheel=remote_wheels_str, config_file=self._config_file).encode("utf8")

tests/unit/test_install.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ def test_create_database(ws, caplog, mock_installation, any_prompt):
228228
raise e.errs[0]
229229

230230
assert "Kindly uninstall and reinstall UCX" in str(failure.value)
231-
wheels.upload_to_wsfs.assert_not_called()
231+
wheels.upload_to_wsfs.assert_called_once()
232232

233233

234234
def test_install_cluster_override_jobs(ws, mock_installation):

0 commit comments

Comments
 (0)