Pivotal ID # 182800115: Folder Submissions handling #590
Conversation
| @@ -63,17 +64,12 @@ internal class FireWebClient( | |||
| } | |||
|
|
|||
| override fun findByMd5(md5: String): List<FireApiFile> = | |||
| template.getForObject<Array<FireApiFile>>("/objects/md5/$md5").toList() | |||
| template.getForObjectOrNull<Array<FireApiFile>>("/objects/md5/$md5").orEmpty().toList() | |||
There was a problem hiding this comment.
Here we're actually calling nonexisting endpoints which are giving us the 404. For example, if the md5 is empty we would hit /objects/md5/ which doesn't exist. Maybe we should not call the endpoints when the values are empty and just return an empty list instead of handling the 404 for a call we know will fail? 🤔
There was a problem hiding this comment.
error on terminal for me , was md5 not found.
There was a problem hiding this comment.
Yeah, I thought it was coming from the standard http error but apparently they decided this design but, at least, I think we could save one request since we know for sure empty md5 or path will not return any result don't you think?
| @@ -25,6 +25,7 @@ dependencies { | |||
| api(project(SubmissionPersistenceCommonApi)) | |||
|
|
|||
| implementation(Arrow) | |||
| implementation("commons-codec:commons-codec:1.15") | |||
There was a problem hiding this comment.
We should add a dependency in Dependencies.kt
| @@ -29,21 +33,47 @@ class FireService( | |||
| * different path) and if so, even if the file exists in FIRE, it gets duplicated to ensure consistency. | |||
| */ | |||
| fun getOrPersist(sub: ExtSubmission, file: ExtFile): FirePersistResult { | |||
| val expectedPath = "/${sub.relPath}/${file.relPath}" | |||
There was a problem hiding this comment.
I had made this change because the paths are the same in both places which I think makes clear that no matter where the file is processed, we use the same path
There was a problem hiding this comment.
yes the problem is that this is not the path when file is a folder. As we need to add the .zip that is the reason I move the call after compressing the file and creating extDirectory.
I did update the code to have single declaration please take a look again.
| return target | ||
| } | ||
|
|
||
| fun calculateMd5(dir: File): String { |
There was a problem hiding this comment.
Since we're always compressing the directory before uploading it to fire, why do we need to calculate the md5 in this way? Or is this the same value as the compressed directory?
I'm just asking because I'm not sure fire will take this md5 for the upload as valid and we'll need to recalculate it anyway and also, if we're trying to find it by md5 in order to reuse it, I'm not sure if this md5 will do
|
|
||
| object ZipUtil { | ||
|
|
||
| fun pack(sourceDir: File, zipFile: File) { |
Enable itest only on FIRE mode
jhoanmanuelms
deleted the
feature/pivotal-182800115-Folder-submissions-handling
branch
https://www.pivotaltracker.com/story/show/182800115