Skip to content

Commit

Permalink
fix(service): return proper errors on migrations check (#3334)
Browse files Browse the repository at this point in the history
  • Loading branch information
Panaetius committed May 31, 2023
1 parent 9cbb465 commit 6237dc7
Show file tree
Hide file tree
Showing 30 changed files with 494 additions and 212 deletions.
38 changes: 28 additions & 10 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion pyproject.toml
Expand Up @@ -107,7 +107,8 @@ zodb = "==5.8.0"
zstandard = ">=0.16.0,<0.22.0"

# service dependencies:
apispec = { version = ">=4.0.0,<5.3.0", optional = true }
apispec = { version = ">=6.3.0,<6.4.0", optional = true }
apispec-oneofschema = { version = ">=3.0.0,<4.0.0", optional = true}
apispec-webframeworks = { version = "<0.6,>=0.5.2", optional = true }
circus = { version = "==0.18.0", optional = true }
flask = { version = "==2.2.5", optional = true }
Expand Down Expand Up @@ -173,6 +174,7 @@ sphinxcontrib-spelling = ">=7,<9"
[tool.poetry.extras]
service = [
"apispec",
"apispec-oneofschema",
"apispec-webframeworks",
"circus",
"flask",
Expand Down Expand Up @@ -282,6 +284,7 @@ check_untyped_defs = true
[[tool.mypy.overrides]]
module = [
"apispec.*",
"apispec_oneofschema.*",
"apispec_webframeworks.*",
"appdirs",
"BTrees.*",
Expand Down
13 changes: 7 additions & 6 deletions renku/command/checks/activities.py
Expand Up @@ -58,7 +58,7 @@ def check_migrated_activity_ids(fix, activity_gateway: IActivityGateway, **_):
wrong_activities = []

if not wrong_activities:
return True, None
return True, False, None

problems = (
WARNING
Expand All @@ -68,7 +68,7 @@ def check_migrated_activity_ids(fix, activity_gateway: IActivityGateway, **_):
+ "\n"
)

return False, problems
return False, True, problems


@inject.autoparams("activity_gateway")
Expand All @@ -81,7 +81,8 @@ def check_activity_dates(fix, activity_gateway: IActivityGateway, **_):
_: keyword arguments.
Returns:
Tuple[bool, Optional[str]]: Tuple of whether there are activities with invalid dates a string of the problem.
Tuple[bool, Optional[str]]: Tuple of whether there are activities with invalid dates, if they can be
automatically fixed and a string of the problem.
"""
invalid_activities = []

Expand All @@ -95,7 +96,7 @@ def check_activity_dates(fix, activity_gateway: IActivityGateway, **_):
invalid_activities.append(activity)

if not invalid_activities:
return True, None
return True, False, None
if not fix:
ids = [a.id for a in invalid_activities]
message = (
Expand All @@ -104,13 +105,13 @@ def check_activity_dates(fix, activity_gateway: IActivityGateway, **_):
+ "\n\t"
+ "\n\t".join(ids)
)
return False, message
return False, True, message

fix_activity_dates(activities=invalid_activities)
project_context.database.commit()
communication.info("Activity dates were fixed")

return True, None
return True, False, None


def fix_activity_dates(activities):
Expand Down
31 changes: 16 additions & 15 deletions renku/command/checks/datasets.py
Expand Up @@ -38,12 +38,13 @@ def check_dataset_old_metadata_location(**_):
_: keyword arguments.
Returns:
Tuple of whether dataset metadata location is valid and string of found problems.
Tuple of whether dataset metadata location is valid, if an automated fix is available and string of
found problems.
"""
old_metadata = get_pre_0_3_4_datasets_metadata()

if not old_metadata:
return True, None
return True, False, None

problems = (
WARNING + "There are metadata files in the old location."
Expand All @@ -52,7 +53,7 @@ def check_dataset_old_metadata_location(**_):
+ "\n"
)

return False, problems
return False, False, problems


@inject.autoparams("dataset_gateway")
Expand All @@ -64,7 +65,7 @@ def check_missing_files(dataset_gateway: IDatasetGateway, **_):
_: keyword arguments.
Returns:
Tuple of whether all dataset files are there and string of found problems.
Tuple of whether all dataset files are there, if an automated fix is available and string of found problems.
"""
missing = defaultdict(list)

Expand All @@ -79,7 +80,7 @@ def check_missing_files(dataset_gateway: IDatasetGateway, **_):
missing[dataset.name].append(file_.entity.path)

if not missing:
return True, None
return True, False, None

problems = WARNING + "There are missing files in datasets."

Expand All @@ -91,7 +92,7 @@ def check_missing_files(dataset_gateway: IDatasetGateway, **_):
+ "\n\t ".join(click.style(path, fg="red") for path in files)
)

return False, problems
return False, False, problems


@inject.autoparams("dataset_gateway")
Expand All @@ -104,7 +105,7 @@ def check_invalid_datasets_derivation(fix, dataset_gateway: IDatasetGateway, **_
_: keyword arguments.
Returns:
Tuple of whether dataset derivations are valid and string of found problems.
Tuple of whether dataset derivations are valid, if an automated fix is available and string of found problems.
"""
invalid_datasets = []

Expand All @@ -130,7 +131,7 @@ def fix_or_report(dataset):
break

if not invalid_datasets:
return True, None
return True, False, None

problems = (
WARNING
Expand All @@ -140,7 +141,7 @@ def fix_or_report(dataset):
+ "\n"
)

return False, problems
return False, True, problems


@inject.autoparams("dataset_gateway")
Expand Down Expand Up @@ -193,9 +194,9 @@ def check_dataset_files_outside_datadir(fix, dataset_gateway: IDatasetGateway, *
+ "\n\t".join(click.style(file.entity.path, fg="yellow") for file in invalid_files)
+ "\n"
)
return False, problems
return False, True, problems

return True, None
return True, False, None


@inject.autoparams("dataset_gateway")
Expand All @@ -208,7 +209,7 @@ def check_external_files(fix, dataset_gateway: IDatasetGateway, **_):
_: keyword arguments.
Returns:
Tuple of whether no external files are found and string of found problems.
Tuple of whether no external files are found, if an automated fix is available and string of found problems.
"""
from renku.core.dataset.dataset import file_unlink

Expand All @@ -222,7 +223,7 @@ def check_external_files(fix, dataset_gateway: IDatasetGateway, **_):
datasets[dataset.name].append(file)

if not external_files:
return True, None
return True, False, None

external_files_str = "\n\t".join(sorted(external_files))

Expand All @@ -232,7 +233,7 @@ def check_external_files(fix, dataset_gateway: IDatasetGateway, **_):
"Use 'renku dataset rm' or rerun 'renku doctor' with '--fix' flag to remove them:\n\t"
f"{external_files_str}\n"
)
return False, problems
return False, True, problems

communication.info(
"The following external files were deleted from the project. You need to add them later manually using a "
Expand All @@ -242,4 +243,4 @@ def check_external_files(fix, dataset_gateway: IDatasetGateway, **_):
for name, files in datasets.items():
file_unlink(name=name, yes=True, dataset_files=files)

return True, None
return True, False, None
8 changes: 4 additions & 4 deletions renku/command/checks/githooks.py
Expand Up @@ -41,7 +41,7 @@ def check_git_hooks_installed(**_):
hook_path = get_hook_path(name=hook, path=project_context.path)
if not hook_path.exists():
message = WARNING + "Git hooks are not installed. " 'Use "renku githooks install" to install them. \n'
return False, message
return False, False, message

with hook_path.open() as file_:
actual_hook = _extract_renku_hook(file_)
Expand All @@ -50,16 +50,16 @@ def check_git_hooks_installed(**_):

if not expected_hook:
message = WARNING + "Cannot check for existence of Git hooks.\n"
return False, message
return False, False, message

if actual_hook != expected_hook:
message = (
WARNING + "Git hooks are outdated or not installed.\n"
' (use "renku githooks install --force" to update them) \n'
)
return False, message
return False, False, message

return True, None
return True, False, None


def _extract_renku_hook(file):
Expand Down
6 changes: 3 additions & 3 deletions renku/command/checks/migration.py
Expand Up @@ -26,7 +26,7 @@ def check_migration(**_):
_: keyword arguments.
Returns:
Tuple of whether project metadata is up to date and string of found problems.
Tuple of whether project metadata is up to date, if an automated fix is available and string of found problems.
"""
if is_migration_required():
problems = WARNING + "Project requires migration.\n" + ' (use "renku migrate" to fix this issue)\n'
Expand All @@ -35,6 +35,6 @@ def check_migration(**_):
ERROR + "Project version is not supported by your version of Renku.\n" + " (upgrade your Renku version)\n"
)
else:
return True, None
return True, False, None

return False, problems
return False, False, problems
20 changes: 12 additions & 8 deletions renku/command/checks/project.py
Expand Up @@ -33,7 +33,7 @@ def check_project_id_group(fix, project_gateway: IProjectGateway, **_):
_: keyword arguments.
Returns:
Tuple of whether project id is valid.
Tuple of whether project id is valid, if an automated fix is available and string of found problems.
"""
current_project = project_gateway.get_project()

Expand All @@ -42,21 +42,25 @@ def check_project_id_group(fix, project_gateway: IProjectGateway, **_):
)

if namespace is None or name is None:
return True, None
return True, False, None

generated_id = Project.generate_id(namespace=namespace, name=name)

if generated_id == current_project.id:
return True, None
return True, False, None

if fix:
communication.info(f"Fixing project id '{current_project.id}' -> '{generated_id}'")
current_project.id = generated_id
project_gateway.update_project(current_project)
return True, None
return True, False, None

return True, (
WARNING
+ "Project id doesn't match id created based on the current Git remote (use 'renku doctor --fix' to fix it):"
f"\n\t'{current_project.id}' -> '{generated_id}'"
return (
False,
True,
(
WARNING
+ "Project id doesn't match id based on the current Git remote (use 'renku doctor --fix' to fix it):"
f"\n\t'{current_project.id}' -> '{generated_id}'"
),
)
10 changes: 5 additions & 5 deletions renku/command/checks/storage.py
Expand Up @@ -26,22 +26,22 @@ def check_lfs_info(**_):
_: keyword arguments.
Returns:
Tuple of whether project structure is valid and string of found problems.
Tuple of whether project structure is valid, if an automated fix is available and string of found problems.
"""
if not check_external_storage():
return True, None
return True, False, None

files = check_lfs_migrate_info()

if not files:
return True, None
return True, False, None

message = (
WARNING
+ "Git history contains large files - consider moving them "
+ "to external storage like git LFS\n\t"
+ "to external storage like git LFS using 'renku storage migrate'\n\t"
+ "\n\t".join(files)
+ "\n"
)

return False, message
return False, False, message

0 comments on commit 6237dc7

Please sign in to comment.