Skip to content

Commit

Permalink
feat(service): restore optimized migration check
Browse files Browse the repository at this point in the history
- handle the GitLab library specific errors.
- fall back to full repository checkout in case of unforseen errors.

fix SwissDataScienceCenter#2546
  • Loading branch information
lorenzo-cavazzi committed Apr 22, 2022
1 parent a665450 commit 7254a91
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 13 deletions.
20 changes: 12 additions & 8 deletions renku/ui/service/controllers/cache_migrations_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from pathlib import Path

from renku.command.migrate import migrations_check
from renku.core.errors import RenkuException
from renku.core.errors import AuthenticationError, ProjectNotFound, RenkuException
from renku.core.util.contexts import click_context
from renku.ui.service.controllers.api.abstract import ServiceCtrl
from renku.ui.service.controllers.api.mixins import RenkuOperationMixin
Expand Down Expand Up @@ -71,11 +71,15 @@ def renku_op(self):

def to_response(self):
"""Execute controller flow and serialize to service response."""
return result_response(self.RESPONSE_SERIALIZER, self.execute_op())
if "project_id" in self.context:
result = self.execute_op()
else:
# NOTE: use quick flow but fallback to regular flow in case of unexpected exceptions
try:
result = self._fast_op_without_cache()
except (AuthenticationError, ProjectNotFound):
raise
except Exception:
result = self.execute_op()

# TODO: Enable this optimization. See https://github.com/SwissDataScienceCenter/renku-python/issues/2546
# if "project_id" in self.context:
# # use regular flow using cache
# return result_response(self.RESPONSE_SERIALIZER, self.execute_op())
#
# return result_response(self.RESPONSE_SERIALIZER, self._fast_op_without_cache())
return result_response(self.RESPONSE_SERIALIZER, result)
2 changes: 1 addition & 1 deletion renku/ui/service/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ class IntermittentAuthenticationError(ServiceError):
This may happen for a number of reasons. Triggering a new login will likely fix it.
"""

code = SVC_ERROR_USER + 30
code = SVC_ERROR_INTERMITTENT + 30
userMessage = "Invalid user credentials. Please try to log out and in again."
devMessage = "Authentication error. Check the Sentry exception to inspect the headers"

Expand Down
32 changes: 29 additions & 3 deletions renku/ui/service/gateways/gitlab_api_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,25 @@

import gitlab

from renku.core import errors
from renku.core.util.os import delete_file
from renku.domain_model.git import GitURL
from renku.ui.service.interfaces.git_api_provider import IGitAPIProvider


class GitlabAPIProvider(IGitAPIProvider):
"""Interface a Git API Provider."""
"""GitLab API provider abstraction layer.
Args:
paths: List of files to download.
target_folder: Folder to use to download the files.
remote: Remote repository URL.
token: User bearer token.
ref: optional reference to checkout,
Raises:
errors.ProjectNotFound: If the remote URL is not accessible.
errors.AuthenticationError: If the bearer token is invalid in any way.
"""

def download_files_from_api(
self,
Expand All @@ -45,8 +57,22 @@ def download_files_from_api(
target_folder = Path(target_folder)

git_data = GitURL.parse(remote)
gl = gitlab.Gitlab(git_data.instance_url, private_token=token)
project = gl.projects.get(f"{git_data.owner}/{git_data.name}")
try:
gl = gitlab.Gitlab(git_data.instance_url, private_token=token)
project = gl.projects.get(f"{git_data.owner}/{git_data.name}")
except gitlab.GitlabAuthenticationError:
# NOTE: Invalid or expired tokens fail even on public projects. Let's give it a try without tokens
try:
gl = gitlab.Gitlab(git_data.instance_url)
project = gl.projects.get(f"{git_data.owner}/{git_data.name}")
except gitlab.GitlabAuthenticationError as e:
raise errors.AuthenticationError from e
except gitlab.GitlabGetError as e:
# NOTE: better to re-raise this as a core error since it's a common case
if "project not found" in getattr(e, "error_message", "").lower():
raise errors.ProjectNotFound from e
else:
raise

result_paths = []

Expand Down
6 changes: 5 additions & 1 deletion renku/ui/service/views/error_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from requests import RequestException

from renku.core.errors import (
AuthenticationError,
DatasetExistsError,
DatasetImageError,
DockerfileUpdateError,
Expand All @@ -35,6 +36,7 @@
MigrationError,
MigrationRequired,
ParameterError,
ProjectNotFound,
RenkuException,
TemplateMissingReferenceError,
TemplateUpdateError,
Expand Down Expand Up @@ -126,7 +128,7 @@ def decorated_function(*args, **kwargs):
"""Represents decorated function."""
try:
return f(*args, **kwargs)
except (ExpiredSignatureError, ImmatureSignatureError, InvalidIssuedAtError) as e:
except (AuthenticationError, ExpiredSignatureError, ImmatureSignatureError, InvalidIssuedAtError) as e:
raise IntermittentAuthenticationError(e)

return decorated_function
Expand Down Expand Up @@ -392,6 +394,8 @@ def decorated_function(*args, **kwargs):
raise UserProjectTemplateReferenceError(e)
except (InvalidTemplateError, TemplateUpdateError) as e:
raise IntermittentProjectTemplateUnavailable(e)
except ProjectNotFound as e:
raise UserRepoUrlInvalidError(e)

return decorated_function

Expand Down
14 changes: 14 additions & 0 deletions tests/service/views/test_cache_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
IntermittentProjectTemplateUnavailable,
UserAnonymousError,
UserProjectTemplateReferenceError,
UserRepoUrlInvalidError,
)
from renku.ui.service.serializers.headers import JWT_TOKEN_SECRET
from tests.utils import assert_rpc_response, retry_failed
Expand Down Expand Up @@ -685,6 +686,19 @@ def test_check_migrations_remote(svc_client, identity_headers, it_remote_repo_ur
assert response.json["result"]["core_renku_version"]


@pytest.mark.service
@pytest.mark.integration
def test_check_migrations_remote_errors(svc_client, identity_headers, it_remote_repo_url):
"""Check if migrations are required for a remote project."""

# NOTE: repo doesn't exist
fake_url = f"{it_remote_repo_url}FAKE_URL"
response = svc_client.get("/cache.migrations_check", query_string=dict(git_url=fake_url), headers=identity_headers)

assert_rpc_response(response, "error")
assert UserRepoUrlInvalidError.code == response.json["error"]["code"]


@pytest.mark.service
@pytest.mark.integration
def test_mirgate_wrong_template_failure(svc_client_with_repo, template, monkeypatch):
Expand Down

0 comments on commit 7254a91

Please sign in to comment.