Skip to content
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

fix(service): return proper errors on migrations check #3334

Merged
merged 20 commits into from May 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
f710119
fix(service): return proper errors on migrations check
Panaetius Feb 17, 2023
39dbf8b
fix tests
Panaetius Feb 27, 2023
59219db
change response format
Panaetius Feb 27, 2023
ace6d40
Merge branch 'develop' into bugfix/2124-fetch-template-errors
Panaetius Feb 27, 2023
a895716
fix test
Panaetius Feb 27, 2023
0a7b441
Merge branch 'develop' into bugfix/2124-fetch-template-errors
Panaetius Mar 7, 2023
e366cae
Merge branch 'develop' into bugfix/2124-fetch-template-errors
Mar 22, 2023
591e27d
don't swallow metadata error
Mar 22, 2023
3225904
test: remove description on nested
Mar 24, 2023
5bd64a9
fix openapi for marshmallow_oneof schemas
Mar 29, 2023
3baf721
Merge branch 'develop' into bugfix/2124-fetch-template-errors
Panaetius Mar 30, 2023
d6f4746
Merge branch 'develop' into bugfix/2124-fetch-template-errors
May 4, 2023
2601875
feat(service): add support for doctor check in cache migration endpoi…
Panaetius May 8, 2023
cdbae4e
Merge branch 'develop' into bugfix/2124-fetch-template-errors
lorenzo-cavazzi May 25, 2023
eb2d4d7
Merge branch 'develop' into bugfix/2124-fetch-template-errors
Panaetius May 25, 2023
fca39f5
chore: add documentation reference to UpdateProject error (#3485)
lorenzo-cavazzi May 26, 2023
580ed9e
Merge branch 'develop' into bugfix/2124-fetch-template-errors
Panaetius May 30, 2023
5708128
Merge branch 'develop' into bugfix/2124-fetch-template-errors
Panaetius May 30, 2023
6344277
address comments
May 31, 2023
0376887
Merge branch 'develop' into bugfix/2124-fetch-template-errors
m-alisafaee May 31, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
Panaetius marked this conversation as resolved.
Show resolved Hide resolved
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
Panaetius marked this conversation as resolved.
Show resolved Hide resolved

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
Panaetius marked this conversation as resolved.
Show resolved Hide resolved

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
Panaetius marked this conversation as resolved.
Show resolved Hide resolved

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
Panaetius marked this conversation as resolved.
Show resolved Hide resolved

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
Panaetius marked this conversation as resolved.
Show resolved Hide resolved

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
Panaetius marked this conversation as resolved.
Show resolved Hide resolved

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
Panaetius marked this conversation as resolved.
Show resolved Hide resolved

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