address PR review comments

helm-chart/renku-core/templates/configmap.yaml

@@ -8,7 +8,7 @@ data:
     upstream {{ $version.name }} {
       server {{ include "renku-core.fullname" $ }}-{{ $version.name }};
       keepalive 32;
-      keepalive_timeout {{ $.Values.requestTimeout }}s;
+      keepalive_timeout 60s;
     }
     {{ end }}
 
@@ -32,7 +32,6 @@ data:
         rewrite /renku/{{ $version.prefix }}/(.*) /renku/$1  break;
         proxy_set_header Host $host;
         proxy_pass http://{{ $version.name }};
-        proxy_connect_timeout {{ $.Values.requestTimeout }}s;
         proxy_send_timeout {{ $.Values.requestTimeout }}s;
         proxy_read_timeout {{ $.Values.requestTimeout }}s;
         proxy_http_version 1.1;
@@ -43,7 +42,6 @@ data:
       location /renku {
         proxy_set_header Host $host;
         proxy_pass http://{{ .Values.versions.latest.name }};
-        proxy_connect_timeout {{ $.Values.requestTimeout }}s;
         proxy_send_timeout {{ $.Values.requestTimeout }}s;
         proxy_read_timeout {{ $.Values.requestTimeout }}s;
         proxy_http_version 1.1;

renku/ui/service/cache/models/file.py

@@ -101,7 +101,7 @@ class FileChunk(Model):
 
     created_at = DateTimeField()
 
-    file_id = TextField(primary_key=True, index=True)
+    chunk_file_id = TextField(primary_key=True, index=True)
     user_id = TextField(index=True)
     chunked_id = TextField(index=True)
 

renku/ui/service/cache/serializers/file.py

@@ -41,7 +41,7 @@ def make_file(self, data, **options):
 class FileChunkSchema(CreationSchema, MandatoryUserSchema):
     """Schema for file model."""
 
-    file_id = fields.String(missing=lambda: uuid.uuid4().hex)
+    chunk_file_id = fields.String(missing=lambda: uuid.uuid4().hex)
     file_name = fields.String(required=True)
 
     chunked_id = fields.String(required=True)

renku/ui/service/controllers/cache_files_delete_chunks.py

@@ -26,16 +26,16 @@
 from renku.ui.service.views import result_response
 
 
-class DeleteFileChunkssCtrl(ServiceCtrl, RenkuOperationMixin):
-    """Controller for listing uploaded files endpoint."""
+class DeleteFileChunksCtrl(ServiceCtrl, RenkuOperationMixin):
+    """Controller for deleting uploaded chunks."""
 
     REQUEST_SERIALIZER = FileChunksDeleteRequest()
     RESPONSE_SERIALIZER = FileChunksDeleteResponseRPC()
 
     def __init__(self, cache, user_data, request_data):
         """Construct list uploaded files controller."""
-        self.ctx = DeleteFileChunkssCtrl.REQUEST_SERIALIZER.load(request_data)
-        super(DeleteFileChunkssCtrl, self).__init__(cache, user_data, request_data)
+        self.ctx = DeleteFileChunksCtrl.REQUEST_SERIALIZER.load(request_data)
+        super(DeleteFileChunksCtrl, self).__init__(cache, user_data, request_data)
 
     @property
     def context(self):
@@ -54,6 +54,8 @@ def delete_chunks(self):
 
         shutil.rmtree(chunks_dir)
 
+        self.cache.invalidate_chunks(self.user, chunked_id)
+
         return f"Deleted chunks for {chunked_id}"
 
     def renku_op(self):
@@ -63,4 +65,4 @@ def renku_op(self):
 
     def to_response(self):
         """Execute controller flow and serialize to service response."""
-        return result_response(DeleteFileChunkssCtrl.RESPONSE_SERIALIZER, self.delete_chunks())
+        return result_response(DeleteFileChunksCtrl.RESPONSE_SERIALIZER, self.delete_chunks())

renku/ui/service/controllers/cache_files_upload.py

@@ -58,6 +58,14 @@ def context(self):
         """Controller operation context."""
         return self.response_builder
 
+    @property
+    def user_cache_dir(self) -> Path:
+        """The cache directory for a user."""
+        directory = CACHE_UPLOADS_PATH / self.user.user_id
+        directory.mkdir(exist_ok=True)
+
+        return directory
+
     def process_upload(self):
         """Process an upload."""
         if self.response_builder.get("chunked_id", None) is None:
@@ -78,15 +86,14 @@ def process_chunked_upload(self):
 
         chunked_id = self.response_builder["chunked_id"]
 
-        user_cache_dir = CACHE_UPLOADS_PATH / self.user.user_id
-        chunks_dir = user_cache_dir / chunked_id
+        chunks_dir: Path = self.user_cache_dir / chunked_id
         chunks_dir.mkdir(exist_ok=True, parents=True)
 
         current_chunk = self.response_builder["chunk_index"]
         total_chunks = self.response_builder["chunk_count"]
 
         file_path = chunks_dir / str(current_chunk)
-        relative_path = file_path.relative_to(CACHE_UPLOADS_PATH / self.user.user_id / chunked_id)
+        relative_path = file_path.relative_to(chunks_dir)
 
         self.file.save(str(file_path))
 
@@ -104,30 +111,30 @@ def process_chunked_upload(self):
         if not completed:
             return {}
 
-        target_file_path = user_cache_dir / self.file.filename
+        target_file_path = self.user_cache_dir / self.file.filename
 
         if target_file_path.exists():
             if self.response_builder.get("override_existing", False):
                 target_file_path.unlink()
             else:
                 raise IntermittentFileExistsError(file_name=self.file.filename)
 
-        with open(target_file_path, "wb") as f:
+        with open(target_file_path, "wb") as target_file:
             for file_number in range(total_chunks):
-                f.write((chunks_dir / str(file_number)).read_bytes())
+                with (chunks_dir / str(file_number)).open("r") as chunk:
+                    shutil.copyfileobj(chunk, target_file)
+
             shutil.rmtree(chunks_dir)
             self.cache.invalidate_chunks(self.user, chunked_id)
 
         self.response_builder["is_archive"] = self.response_builder.get("chunked_content_type") in SUPPORTED_ARCHIVES
 
-        return self.postprocess_file(target_file_path, user_cache_dir)
+        return self.postprocess_file(target_file_path)
 
     def process_file(self):
         """Process uploaded file."""
-        user_cache_dir = CACHE_UPLOADS_PATH / self.user.user_id
-        user_cache_dir.mkdir(exist_ok=True)
 
-        file_path = user_cache_dir / self.file.filename
+        file_path = self.user_cache_dir / self.file.filename
         if file_path.exists():
             if self.response_builder.get("override_existing", False):
                 file_path.unlink()
@@ -136,9 +143,9 @@ def process_file(self):
 
         self.file.save(str(file_path))
 
-        return self.postprocess_file(file_path, user_cache_dir)
+        return self.postprocess_file(file_path)
 
-    def postprocess_file(self, file_path, user_cache_dir):
+    def postprocess_file(self, file_path):
         """Postprocessing of uploaded file."""
         files = []
         if self.response_builder["unpack_archive"] and self.response_builder["is_archive"]:
@@ -154,7 +161,7 @@ def postprocess_file(self, file_path, user_cache_dir):
                 return RenkuException("unable to unpack archive")
 
             for file_ in temp_dir.glob("**/*"):
-                relative_path = file_.relative_to(user_cache_dir)
+                relative_path = file_.relative_to(self.user_cache_dir)
 
                 file_obj = {
                     "file_name": file_.name,
@@ -166,7 +173,7 @@ def postprocess_file(self, file_path, user_cache_dir):
                 files.append(file_obj)
 
         else:
-            relative_path = file_path.relative_to(CACHE_UPLOADS_PATH / self.user.user_id)
+            relative_path = file_path.relative_to(self.user_cache_dir)
 
             file_obj = {
                 "file_name": self.response_builder["file_name"],

renku/ui/service/views/cache.py

@@ -19,7 +19,7 @@
 from flask import request
 
 from renku.ui.service.config import SERVICE_PREFIX
-from renku.ui.service.controllers.cache_files_delete_chunks import DeleteFileChunkssCtrl
+from renku.ui.service.controllers.cache_files_delete_chunks import DeleteFileChunksCtrl
 from renku.ui.service.controllers.cache_files_upload import UploadFilesCtrl
 from renku.ui.service.controllers.cache_list_projects import ListProjectsCtrl
 from renku.ui.service.controllers.cache_list_uploaded import ListUploadedFilesCtrl
@@ -125,7 +125,7 @@ def delete_file_chunks_view(user_data, cache):
       tags:
         - cache
     """
-    return DeleteFileChunkssCtrl(cache, user_data, dict(request.json)).to_response()
+    return DeleteFileChunksCtrl(cache, user_data, dict(request.json)).to_response()
 
 
 @cache_blueprint.route("/cache.project_clone", methods=["POST"], provide_automatic_options=False, versions=ALL_VERSIONS)

tests/service/views/test_cache_views.py

@@ -240,7 +240,7 @@ def test_file_chunked_upload_zipped(svc_client, identity_headers, svc_cache_dir)
 
 @pytest.mark.service
 def test_file_chunked_upload_delete(svc_client, identity_headers, svc_cache_dir):
-    """Check successful file upload."""
+    """Test deleting uploaded file chunks."""
     headers = copy.deepcopy(identity_headers)
     content_type = headers.pop("Content-Type")