Add endpoint to update previously registered S3 files #4893
Conversation
| pathPrefix("register-update") { | ||
| (idSegment & indexingMode) { (id, mode) => | ||
| pathEndOrSingleSlash { | ||
| operationName(s"$prefixSegment/files/{org}/{project}/register-update/{id}") { |
There was a problem hiding this comment.
Not important but we don't need this operationName anymore as the Kamon feature is disabled because it made Prometheus crash by sending data with a lot of distinct values (which Prometheus is not fond about)
| id: FileId, | ||
| storageId: Option[IdSegment], | ||
| rev: Int, | ||
| entity: HttpEntity, |
There was a problem hiding this comment.
Why do we have a http entity here ?
It should be a path like the registerFile no ?
| @@ -290,6 +290,32 @@ final class FilesRoutes( | |||
| } | |||
| } | |||
| } | |||
| }, | |||
| pathPrefix("register-update") { | |||
There was a problem hiding this comment.
Can be fixed in a later PR but we have a problem, it clashes with this endpoint:
https://bluebrainnexus.io/docs/delta/api/files-api.html#create-using-put
There was a problem hiding this comment.
Not in the update operation but for the create one, we will need to provide a way to "register" a file where the user leaves Delta generating the id
There was a problem hiding this comment.
Ah yeah this was intentional - can we do without the version where we generate the Id? Fewer endpoints => smaller API surface => less maintenance (tests, docs, code, etc)
| @@ -138,6 +137,9 @@ object StorageFileRejection { | |||
| s"File cannot be saved because it already exists on path '$path'." | |||
| ) | |||
|
|
|||
| final case class FileNotFound(path: String) | |||
There was a problem hiding this comment.
I don't know if we can rename it in a way that makes it different from the FileNotFound from FileRejection ?
|
|
||
| def overwriteFile( |
There was a problem hiding this comment.
The physical files should not be overwritte, updating a file means a new revision in the primary store and a new physical file.
When updating a file through register, we should just provide a path to another file which should already exist in S3
@olivergrabinski is this enough to unblock you? Still need to verify all the file attribute fields in the E2E test response and the API design could be better.
Some points of note
FileAttributesOriginfromExternaltoClient.