Pivotal ID # 184975504: S3 submission resubmission #702
Conversation
| assertThat(webClient.submitSingle(submission, TSV, FIRE)).isSuccessful() | ||
| val fireSub = submissionRepository.getExtByAccNo("S-STR-MODE-4", includeFileListFiles = true) | ||
|
|
||
| // We upload files manually into S3 to simulate fire S3 support. |
There was a problem hiding this comment.
Just to clarify, is this to validate the case where the imaging team uploads the files directly to FIRE? If so, I don't think this is the right place to validate this logic since the idea of the transfer function is for submissions that already exist and will be transferred as-is so it seems weird to me we're adding files afterward.
I think the proper test case for this would be here like this:
- Submit to FIRE
- Upload extra files manually
- Resubmit to NFS including the manually uploaded files in the new version
There was a problem hiding this comment.
No, this is not because of that. This changes are because we need to simulate fire mechanism that make every file uploaded through http it make it avaiable in s3.
There was a problem hiding this comment.
wondering if the comment need to be better to clarify this
|
|
||
| internal const val EXCEL_FILE_REQUIRED = "Excel file is required for Eu-ToxRisk submissions" | ||
| internal const val SKIP_VALIDATION_ATTR = "QMRF-ID" | ||
|
|
||
| class EuToxRiskValidator( | ||
| private val restTemplate: RestTemplate, | ||
| private val validationProperties: ValidatorProperties | ||
| private val validationProperties: ValidatorProperties, | ||
| private val fireClient: FireClient, |
| @@ -25,11 +26,6 @@ internal class RetryWebClient( | |||
| template.execute(opt) { fireClient.unsetPath(fireOid) } | |||
| } | |||
|
|
|||
| override fun downloadByPath(path: String): File? { | |||
| val opt = "Download file path='$path'" | |||
| return template.execute(opt) { fireClient.downloadByPath(path) } | |||
There was a problem hiding this comment.
Since we're removing this, I think we should also remove the http client method and the related tests since it won't be required anymore
| import java.nio.file.Files | ||
| import java.nio.file.StandardCopyOption | ||
|
|
||
| class S3Client( |
There was a problem hiding this comment.
I think we're missing unit tests for this client
…services into feature/pivotal-184975504-S3-submission-resubmission
https://www.pivotaltracker.com/story/show/184975504