Pivotal ID # 184462975: Allow DB by pass even when not FIRE prefered source #682
Conversation
| import java.io.File | ||
|
|
||
| /** | ||
| * Source that allows submitted to use store files directly in fire and bypass backend mechanism. |
There was a problem hiding this comment.
typo: "allows the submitter"
| override val description: String | ||
| get() = "Provided Db files" | ||
|
|
||
| override fun getExtFile(path: String, dbFile: DbFile?, attributes: List<Attribute>): ExtFile? { |
There was a problem hiding this comment.
I understand this change and I think it makes sense, however, if we're to separate these type of files as yet another source, I think we should also decouple the responsability of passing around the DbFile but instead, treat this source like the others which are just a list of files, and populate it in the FilesSourceService
The reason why I'm suggesting this is because to me it doesn't make sense to create another type of list just to validate if a given parameter is of a given type but it should just give me the file. I think it actually makes more sense as it's right now since it gives us the understanding that there're are two types of fire files and we handle it all in the FireSource.
In summary, I think we should either leave it as it's or treat the DbFile as a separate type of files, properly populate it and refer to it just by path like in the other sources.
Sorry about the long message 😅
There was a problem hiding this comment.
great suggestion man, defenitly require way more changes but is the right way of desing and modeling this.
| add(FilesListSource(files)) | ||
| } | ||
|
|
||
| if (submitter.superuser) add(DbFilesSource) |
There was a problem hiding this comment.
This is what I'm talking about, instead of just adding the object, we should add the list of any DbFiles present in the submission
There was a problem hiding this comment.
but they can be a millions files, we by pass specifically for that reason
There was a problem hiding this comment.
With your current implementation, we wouldn't need to have them all in memory but we would figure it out at runtime.
| @@ -193,7 +195,7 @@ class ChangeLog007 { | |||
| @ChangeLog | |||
| class ChangeLog008 { | |||
| @ChangeSet(order = "008", id = "File List files index", author = "System") | |||
| fun changeSet007(template: MongockTemplate) { | |||
| fun changeSet008(template: MongockTemplate) { | |||
|
|
||
| val result = testInstance.fileSequence(submission).toList() | ||
|
|
||
| assertThat(filesCount).isEqualTo(997) |
There was a problem hiding this comment.
Shouldn't we use files.size() rather than a fixed number?
| @@ -24,7 +24,8 @@ class FireFilesSource( | |||
| ): ExtFile? { | |||
| return when (dbFile) { | |||
| null -> null | |||
| else -> fireClient.findByDb(dbFile, path, attributes) | |||
| is ConfiguredDbFile -> null | |||
There was a problem hiding this comment.
Following the thread above, we wouldn't really need this distinction since this source would only contain FIRE files and the other DbFiles (bypassed files)
| target: OutputStream, | ||
| ) { | ||
| val sourceFiles = serializationService.deserializeFileList(input, format) | ||
| .onEachIndexed { idx, bioFile -> logger.info { "$accNo, Mapping file $idx, path='${bioFile.path}'" } } |
There was a problem hiding this comment.
Nice change, I wonder if we should apply the same validation to this endpoint so we don't have two separate validation workflows
…services into feature/pivotal-184462975-Allow-DB-by-pass-even-when-not-FIRE-prefered-source
| override fun getFile(path: String): File? = null | ||
|
|
||
| @Suppress("ComplexCondition") | ||
| private fun getDbFile(attributes: Map<String, String?>): FireByPassFile? { |
There was a problem hiding this comment.
Clever implementation bro 🤓
| add(FilesListSource(files)) | ||
| } | ||
|
|
||
| if (submitter.superuser) add(DbFilesSource) |
There was a problem hiding this comment.
With your current implementation, we wouldn't need to have them all in memory but we would figure it out at runtime.
Juan-EBI
deleted the
feature/pivotal-184462975-Allow-DB-by-pass-even-when-not-FIRE-prefered-source
branch
https://www.pivotaltracker.com/story/show/184462975