Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
m-alisafaee committed Jun 9, 2022
1 parent 7feff9d commit c397fe2
Show file tree
Hide file tree
Showing 14 changed files with 66 additions and 72 deletions.
6 changes: 3 additions & 3 deletions renku/core/dataset/dataset_add.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
from renku.core.interface.database_dispatcher import IDatabaseDispatcher
from renku.core.util import communication, requests
from renku.core.util.git import clone_repository, get_cache_directory_for_repository, get_git_user
from renku.core.util.metadata import is_external_file, make_temp_dir
from renku.core.util.metadata import is_external_file, make_project_temp_dir
from renku.core.util.os import delete_file, get_absolute_path, get_files, get_relative_path, is_subpath
from renku.core.util.urls import check_url, provider_check, remove_credentials
from renku.domain_model.dataset import Dataset, DatasetFile, RemoteEntity, get_dataset_data_dir
Expand Down Expand Up @@ -566,15 +566,15 @@ def add_file(src_entity_path: str, content_path: Path, checksum) -> None:
for file in sources:
add_file(file, content_path=repository.path / file, checksum=None)
else: # NOTE: Renku dataset import with a tag
content_path_root = make_temp_dir(client.path)
content_path_root = make_project_temp_dir(client.path)
content_path_root.mkdir(parents=True, exist_ok=True)
filename = 1

# NOTE: This is required to enable LFS filters when getting file content
repository.install_lfs(skip_smudge=False) # type: ignore
# NOTE: Git looks at the current attributes files when loading LFS files which won't includes deleted files, so,
# we need to include all files that were in LFS at some point
git_attributes = repository.get_changes_patch(".gitattributes")
git_attributes = repository.get_historical_changes_patch(".gitattributes")
all_additions = [a.replace("+", "", 1) for a in git_attributes if a.startswith("+") and "filter=lfs" in a]
(repository.path / ".gitattributes").write_text(os.linesep.join(all_additions))

Expand Down
2 changes: 1 addition & 1 deletion renku/core/dataset/providers/olos.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def set_access_token(self, access_token):

def access_token_url(self):
"""Endpoint for creation of access token."""
return urllib.parse.urljoin(self.server_url, "/portal by clicking on the top-right menu and selecting 'token'")
return urllib.parse.urljoin(self.server_url, "portal")

def export(self, publish, client=None, **kwargs):
"""Execute export process."""
Expand Down
5 changes: 1 addition & 4 deletions renku/core/dataset/tag.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,7 @@ def prompt_access_token(exporter):
Returns:
The new access token
"""
text_prompt = "You must configure an access token\n"
text_prompt += "Create one at: {0}\n".format(exporter.access_token_url())
text_prompt += "Access token"

text_prompt = f"You must configure an access token\nCreate one at: {exporter.access_token_url()}\nAccess token: "
return communication.prompt(text_prompt, type=str)


Expand Down
2 changes: 1 addition & 1 deletion renku/core/util/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def read_renku_version_from_dockerfile(path: Union[Path, str]) -> Optional[str]:
return None


def make_temp_dir(client_path: Path) -> Path:
def make_project_temp_dir(client_path: Path) -> Path:
"""Create a temporary directory inside project's temp path."""
base = client_path / RENKU_HOME / RENKU_TMP
base.mkdir(parents=True, exist_ok=True)
Expand Down
22 changes: 16 additions & 6 deletions renku/infrastructure/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ def __exit__(self, *args):
self.close()

def __del__(self):
# self.close()
pass
self.close()

@property
def path(self) -> Path:
Expand Down Expand Up @@ -197,7 +196,7 @@ def install_lfs(self, skip_smudge: bool = True):
except errors.GitCommandError as e:
raise errors.GitError(f"Cannot install Git LFS in {self}") from e

def get_changes_patch(self, path: Union[Path, str]) -> List[str]:
def get_historical_changes_patch(self, path: Union[Path, str]) -> List[str]:
"""Return a patch of all changes to a file."""
return self.run_git_command("log", "--", str(path), patch=True, follow=True).splitlines()

Expand Down Expand Up @@ -488,8 +487,20 @@ def copy_content_to_file(
) -> str:
"""Get content of an object using its checksum, write it to a file, and return the file's path.
NOTE: ``apply_filters`` doesn't smudge LFS objects if repository is cloned with ``--skip-smudge`` or if
``GIT_LFS_SKIP_SMUDGE`` is set. Call ``Repository.install_lfs`` before to make sure that LFS objects are pulled.
Args:
path(Union[Path, str]): Relative or absolute path to the file.
revision(Optional[Union["Reference", str]]): A commit/branch/tag to get the file from. This cannot be passed
with ``checksum``.
checksum(Optional[str]): Git hash of the file to be retrieved. This cannot be passed with ``revision``.
output_path(Optional[Union[Path, str]]): A path to copy the content to. A temporary file is created if it is
``None``.
apply_filters(bool): Whether to apply Git filter on the retrieved object.Note that ``apply_filters`` doesn't
smudge LFS objects if repository is cloned with ``--skip-smudge`` or if ``GIT_LFS_SKIP_SMUDGE`` is set.
Call ``Repository.install_lfs`` before to make sure that LFS objects are pulled.
Returns:
The path to the created file.
"""
absolute_path = get_absolute_path(path, self.path)

Expand Down Expand Up @@ -701,7 +712,6 @@ def close(self) -> None:
Cleans up dangling processes.
"""
if getattr(self, "_repository", None) is not None:
self._repository.close() # type:ignore
del self._repository
self._repository = None

Expand Down
9 changes: 7 additions & 2 deletions renku/ui/service/views/api_versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,12 @@ def add_url_rule(

V0_9 = ApiVersion("0.9")
V1_0 = ApiVersion("1.0")
V1_1 = ApiVersion("1.1", is_base_version=True)
V1_1 = ApiVersion("1.1")
V1_2 = ApiVersion("1.2", is_base_version=True)

ALL_VERSIONS = [V0_9, V1_0, V1_1, V1_2]
VERSIONS_FROM_V1_0 = [V1_0, V1_1, V1_2]
VERSIONS_FROM_V1_1 = [V1_1, V1_2]

MINIMUM_VERSION = V0_9
MAXIMUM_VERSION = V1_1
MAXIMUM_VERSION = V1_2
22 changes: 7 additions & 15 deletions renku/ui/service/views/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from renku.ui.service.controllers.cache_migrations_check import MigrationsCheckCtrl
from renku.ui.service.controllers.cache_project_clone import ProjectCloneCtrl
from renku.ui.service.gateways.gitlab_api_provider import GitlabAPIProvider
from renku.ui.service.views.api_versions import V0_9, V1_0, V1_1, VersionedBlueprint
from renku.ui.service.views.api_versions import ALL_VERSIONS, VERSIONS_FROM_V1_0, VERSIONS_FROM_V1_1, VersionedBlueprint
from renku.ui.service.views.decorators import accepts_json, optional_identity, requires_cache, requires_identity
from renku.ui.service.views.error_handlers import (
handle_common_except,
Expand All @@ -40,9 +40,7 @@
cache_blueprint = VersionedBlueprint("cache", __name__, url_prefix=SERVICE_PREFIX)


@cache_blueprint.route(
"/cache.files_list", methods=["GET"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1]
)
@cache_blueprint.route("/cache.files_list", methods=["GET"], provide_automatic_options=False, versions=ALL_VERSIONS)
@handle_common_except
@requires_cache
@requires_identity
Expand All @@ -65,9 +63,7 @@ def list_uploaded_files_view(user_data, cache):
return ListUploadedFilesCtrl(cache, user_data).to_response()


@cache_blueprint.route(
"/cache.files_upload", methods=["POST"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1]
)
@cache_blueprint.route("/cache.files_upload", methods=["POST"], provide_automatic_options=False, versions=ALL_VERSIONS)
@handle_common_except
@requires_cache
@requires_identity
Expand Down Expand Up @@ -102,9 +98,7 @@ def upload_file_view(user_data, cache):
return UploadFilesCtrl(cache, user_data, request).to_response()


@cache_blueprint.route(
"/cache.project_clone", methods=["POST"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1]
)
@cache_blueprint.route("/cache.project_clone", methods=["POST"], provide_automatic_options=False, versions=ALL_VERSIONS)
@handle_common_except
@accepts_json
@requires_cache
Expand Down Expand Up @@ -133,9 +127,7 @@ def project_clone_view(user_data, cache):
return ProjectCloneCtrl(cache, user_data, dict(request.json)).to_response()


@cache_blueprint.route(
"/cache.project_list", methods=["GET"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1]
)
@cache_blueprint.route("/cache.project_list", methods=["GET"], provide_automatic_options=False, versions=ALL_VERSIONS)
@handle_common_except
@requires_cache
@requires_identity
Expand All @@ -158,7 +150,7 @@ def list_projects_view(user_data, cache):
return ListProjectsCtrl(cache, user_data).to_response()


@cache_blueprint.route("/cache.migrate", methods=["POST"], provide_automatic_options=False, versions=[V1_1])
@cache_blueprint.route("/cache.migrate", methods=["POST"], provide_automatic_options=False, versions=VERSIONS_FROM_V1_1)
@handle_common_except
@handle_migration_write_errors
@accepts_json
Expand Down Expand Up @@ -188,7 +180,7 @@ def migrate_project_view(user_data, cache):


@cache_blueprint.route(
"/cache.migrations_check", methods=["GET"], provide_automatic_options=False, versions=[V1_0, V1_1]
"/cache.migrations_check", methods=["GET"], provide_automatic_options=False, versions=VERSIONS_FROM_V1_0
)
@handle_common_except
@handle_migration_read_errors
Expand Down
6 changes: 3 additions & 3 deletions renku/ui/service/views/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from renku.ui.service.config import SERVICE_PREFIX
from renku.ui.service.controllers.config_set import SetConfigCtrl
from renku.ui.service.controllers.config_show import ShowConfigCtrl
from renku.ui.service.views.api_versions import V0_9, V1_0, V1_1, VersionedBlueprint
from renku.ui.service.views.api_versions import ALL_VERSIONS, VersionedBlueprint
from renku.ui.service.views.decorators import accepts_json, optional_identity, requires_cache, requires_identity
from renku.ui.service.views.error_handlers import (
handle_common_except,
Expand All @@ -33,7 +33,7 @@
config_blueprint = VersionedBlueprint("config", __name__, url_prefix=SERVICE_PREFIX)


@config_blueprint.route("/config.show", methods=["GET"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1])
@config_blueprint.route("/config.show", methods=["GET"], provide_automatic_options=False, versions=ALL_VERSIONS)
@handle_common_except
@handle_config_read_errors
@requires_cache
Expand All @@ -60,7 +60,7 @@ def show_config(user_data, cache):
return ShowConfigCtrl(cache, user_data, dict(request.args)).to_response()


@config_blueprint.route("/config.set", methods=["POST"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1])
@config_blueprint.route("/config.set", methods=["POST"], provide_automatic_options=False, versions=ALL_VERSIONS)
@handle_common_except
@handle_config_write_errors
@accepts_json
Expand Down
32 changes: 9 additions & 23 deletions renku/ui/service/views/datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from renku.ui.service.controllers.datasets_list import DatasetsListCtrl
from renku.ui.service.controllers.datasets_remove import DatasetsRemoveCtrl
from renku.ui.service.controllers.datasets_unlink import DatasetsUnlinkCtrl
from renku.ui.service.views.api_versions import V0_9, V1_0, V1_1, VersionedBlueprint
from renku.ui.service.views.api_versions import ALL_VERSIONS, VersionedBlueprint
from renku.ui.service.views.decorators import accepts_json, optional_identity, requires_cache, requires_identity
from renku.ui.service.views.error_handlers import (
handle_common_except,
Expand All @@ -39,9 +39,7 @@
dataset_blueprint = VersionedBlueprint(DATASET_BLUEPRINT_TAG, __name__, url_prefix=SERVICE_PREFIX)


@dataset_blueprint.route(
"/datasets.list", methods=["GET"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1]
)
@dataset_blueprint.route("/datasets.list", methods=["GET"], provide_automatic_options=False, versions=ALL_VERSIONS)
@handle_common_except
@requires_cache
@optional_identity
Expand All @@ -68,7 +66,7 @@ def list_datasets_view(user_data, cache):


@dataset_blueprint.route(
"/datasets.files_list", methods=["GET"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1]
"/datasets.files_list", methods=["GET"], provide_automatic_options=False, versions=ALL_VERSIONS
)
@handle_common_except
@requires_cache
Expand All @@ -95,9 +93,7 @@ def list_dataset_files_view(user_data, cache):
return DatasetsFilesListCtrl(cache, user_data, dict(request.args)).to_response()


@dataset_blueprint.route(
"/datasets.add", methods=["POST"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1]
)
@dataset_blueprint.route("/datasets.add", methods=["POST"], provide_automatic_options=False, versions=ALL_VERSIONS)
@handle_common_except
@handle_datasets_write_errors
@accepts_json
Expand Down Expand Up @@ -126,9 +122,7 @@ def add_file_to_dataset_view(user_data, cache):
return DatasetsAddFileCtrl(cache, user_data, dict(request.json)).to_response()


@dataset_blueprint.route(
"/datasets.create", methods=["POST"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1]
)
@dataset_blueprint.route("/datasets.create", methods=["POST"], provide_automatic_options=False, versions=ALL_VERSIONS)
@handle_common_except
@handle_datasets_write_errors
@accepts_json
Expand Down Expand Up @@ -157,9 +151,7 @@ def create_dataset_view(user_data, cache):
return DatasetsCreateCtrl(cache, user_data, dict(request.json)).to_response()


@dataset_blueprint.route(
"/datasets.remove", methods=["POST"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1]
)
@dataset_blueprint.route("/datasets.remove", methods=["POST"], provide_automatic_options=False, versions=ALL_VERSIONS)
@handle_common_except
@accepts_json
@requires_cache
Expand Down Expand Up @@ -187,9 +179,7 @@ def remove_dataset_view(user_data, cache):
return DatasetsRemoveCtrl(cache, user_data, dict(request.json)).to_response()


@dataset_blueprint.route(
"/datasets.import", methods=["POST"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1]
)
@dataset_blueprint.route("/datasets.import", methods=["POST"], provide_automatic_options=False, versions=ALL_VERSIONS)
@handle_common_except
@accepts_json
@requires_cache
Expand Down Expand Up @@ -217,9 +207,7 @@ def import_dataset_view(user_data, cache):
return DatasetsImportCtrl(cache, user_data, dict(request.json)).to_response()


@dataset_blueprint.route(
"/datasets.edit", methods=["POST"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1]
)
@dataset_blueprint.route("/datasets.edit", methods=["POST"], provide_automatic_options=False, versions=ALL_VERSIONS)
@handle_common_except
@handle_datasets_write_errors
@accepts_json
Expand Down Expand Up @@ -248,9 +236,7 @@ def edit_dataset_view(user_data, cache):
return DatasetsEditCtrl(cache, user_data, dict(request.json)).to_response()


@dataset_blueprint.route(
"/datasets.unlink", methods=["POST"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1]
)
@dataset_blueprint.route("/datasets.unlink", methods=["POST"], provide_automatic_options=False, versions=ALL_VERSIONS)
@handle_common_except
@handle_datasets_unlink_errors
@accepts_json
Expand Down
4 changes: 2 additions & 2 deletions renku/ui/service/views/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@

from renku.ui.service.config import SERVICE_PREFIX
from renku.ui.service.controllers.graph_export import GraphExportCtrl
from renku.ui.service.views.api_versions import V1_0, V1_1, VersionedBlueprint
from renku.ui.service.views.api_versions import VERSIONS_FROM_V1_0, VersionedBlueprint
from renku.ui.service.views.decorators import accepts_json, optional_identity, requires_cache
from renku.ui.service.views.error_handlers import handle_common_except, handle_graph_errors

GRAPH_BLUEPRINT_TAG = "graph"
graph_blueprint = VersionedBlueprint(GRAPH_BLUEPRINT_TAG, __name__, url_prefix=SERVICE_PREFIX)


@graph_blueprint.route("/graph.export", methods=["GET"], provide_automatic_options=False, versions=[V1_0, V1_1])
@graph_blueprint.route("/graph.export", methods=["GET"], provide_automatic_options=False, versions=VERSIONS_FROM_V1_0)
@handle_common_except
@handle_graph_errors
@requires_cache
Expand Down
6 changes: 3 additions & 3 deletions renku/ui/service/views/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@
from renku.ui.service.errors import IntermittentProjectIdError
from renku.ui.service.serializers.jobs import JobDetailsResponseRPC, JobListResponseRPC
from renku.ui.service.views import result_response
from renku.ui.service.views.api_versions import V0_9, V1_0, V1_1, VersionedBlueprint
from renku.ui.service.views.api_versions import ALL_VERSIONS, VersionedBlueprint
from renku.ui.service.views.decorators import requires_cache, requires_identity
from renku.ui.service.views.error_handlers import handle_common_except

JOBS_BLUEPRINT_TAG = "jobs"
jobs_blueprint = VersionedBlueprint("jobs", __name__, url_prefix=SERVICE_PREFIX)


@jobs_blueprint.route("/jobs", methods=["GET"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1])
@jobs_blueprint.route("/jobs", methods=["GET"], provide_automatic_options=False, versions=ALL_VERSIONS)
@handle_common_except
@requires_cache
@requires_identity
Expand Down Expand Up @@ -64,7 +64,7 @@ def list_jobs(user_data, cache):
return result_response(JobListResponseRPC(), {"jobs": jobs})


@jobs_blueprint.route("/jobs/<job_id>", methods=["GET"], provide_automatic_options=False, versions=[V0_9, V1_0, V1_1])
@jobs_blueprint.route("/jobs/<job_id>", methods=["GET"], provide_automatic_options=False, versions=ALL_VERSIONS)
@handle_common_except
@requires_cache
@requires_identity
Expand Down
12 changes: 8 additions & 4 deletions renku/ui/service/views/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,17 @@
from renku.ui.service.controllers.project_edit import ProjectEditCtrl
from renku.ui.service.controllers.project_lock_status import ProjectLockStatusCtrl
from renku.ui.service.controllers.project_show import ProjectShowCtrl
from renku.ui.service.views.api_versions import V1_0, V1_1, VersionedBlueprint
from renku.ui.service.views.api_versions import VERSIONS_FROM_V1_0, VersionedBlueprint
from renku.ui.service.views.decorators import accepts_json, requires_cache, requires_identity
from renku.ui.service.views.error_handlers import handle_common_except, handle_project_write_errors

PROJECT_BLUEPRINT_TAG = "project"
project_blueprint = VersionedBlueprint(PROJECT_BLUEPRINT_TAG, __name__, url_prefix=SERVICE_PREFIX)


@project_blueprint.route("/project.show", methods=["POST"], provide_automatic_options=False, versions=[V1_0, V1_1])
@project_blueprint.route(
"/project.show", methods=["POST"], provide_automatic_options=False, versions=VERSIONS_FROM_V1_0
)
@handle_common_except
@accepts_json
@requires_cache
Expand Down Expand Up @@ -58,7 +60,9 @@ def show_project_view(user_data, cache):
return ProjectShowCtrl(cache, user_data, dict(request.json)).to_response()


@project_blueprint.route("/project.edit", methods=["POST"], provide_automatic_options=False, versions=[V1_0, V1_1])
@project_blueprint.route(
"/project.edit", methods=["POST"], provide_automatic_options=False, versions=VERSIONS_FROM_V1_0
)
@handle_common_except
@handle_project_write_errors
@accepts_json
Expand Down Expand Up @@ -88,7 +92,7 @@ def edit_project_view(user_data, cache):


@project_blueprint.route(
"/project.lock_status", methods=["GET"], provide_automatic_options=False, versions=[V1_0, V1_1]
"/project.lock_status", methods=["GET"], provide_automatic_options=False, versions=VERSIONS_FROM_V1_0
)
@handle_common_except
@requires_cache
Expand Down

0 comments on commit c397fe2

Please sign in to comment.