Skip to content

Commit

Permalink
feat(service): restore optimized migration check (#2854)
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 unforeseen errors.

fix #2546
  • Loading branch information
lorenzo-cavazzi committed Apr 27, 2022
1 parent fe1c2c7 commit 7e2a3d4
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 16 deletions.
20 changes: 12 additions & 8 deletions renku/ui/service/controllers/cache_migrations_check.py
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 BaseException:
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
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
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
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
8 changes: 5 additions & 3 deletions tests/fixtures/config.py
Expand Up @@ -21,12 +21,14 @@

import pytest

IT_REMOTE_REPO_URL = os.getenv(
"IT_REMOTE_REPOSITORY", "https://dev.renku.ch/gitlab/renku-python-integration-tests/core-integration-test"
)
IT_PROTECTED_REMOTE_REPO_URL = os.getenv(
"IT_PROTECTED_REMOTE_REPO", "https://dev.renku.ch/gitlab/renku-python-integration-tests/core-it-protected.git"
)

IT_REMOTE_REPO_URL = os.getenv(
"IT_REMOTE_REPOSITORY", "https://dev.renku.ch/gitlab/renku-python-integration-tests/core-integration-test"
IT_PUBLIC_REMOTE_REPO_URL = os.getenv(
"IT_PUBLIC_REMOTE_REPO", "https://dev.renku.ch/gitlab/renku-python-integration-tests/core-it-public"
)
IT_REMOTE_NON_RENKU_REPO_URL = os.getenv(
"IT_REMOTE_NON_RENKU_REPO_URL", "https://dev.renku.ch/gitlab/renku-python-integration-tests/core-it-non-renku"
Expand Down
8 changes: 8 additions & 0 deletions tests/service/fixtures/service_projects.py
Expand Up @@ -63,6 +63,14 @@ def it_remote_repo_url():
return IT_REMOTE_REPO_URL


@pytest.fixture(scope="module")
def it_remote_public_renku_repo_url():
"""Returns a remote path to a public integration test repository."""
from tests.fixtures.config import IT_PUBLIC_REMOTE_REPO_URL

return IT_PUBLIC_REMOTE_REPO_URL


@pytest.fixture(scope="module")
def it_remote_public_repo_url():
"""Returns a remote path to a public integration test repository."""
Expand Down
34 changes: 34 additions & 0 deletions tests/service/views/test_cache_views.py
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,39 @@ 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, it_remote_public_renku_repo_url
):
"""Check migrations throws the correct errors."""

# 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"]

# NOTE: token errors
response = svc_client.get(
"/cache.migrations_check", query_string=dict(git_url=it_remote_repo_url), headers=identity_headers
)
assert_rpc_response(response)

identity_headers["Authorization"] = "Bearer 123abc"
response = svc_client.get(
"/cache.migrations_check", query_string=dict(git_url=it_remote_repo_url), headers=identity_headers
)
assert_rpc_response(response, "error")
assert UserRepoUrlInvalidError.code == response.json["error"]["code"]

response = svc_client.get(
"/cache.migrations_check", query_string=dict(git_url=it_remote_public_renku_repo_url), headers=identity_headers
)
assert_rpc_response(response)


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

0 comments on commit 7e2a3d4

Please sign in to comment.