[Clean up] deletion of the full MINC header entry from parameter_file as not used anymore #4925
Conversation
cmadjar
added
Add to Release Notes
|
@ridz1208 I will need to update the RB documentation to remove the query to delete that |
jesscall
added
the
Passed Manual Tests
|
This seems like a pretty major change to me since there might be users depending on that data being in the database. |
|
It feels like we are going in circle with this one. Didn't we say that this specific header was completely useless and not queryable anyway at the last imaging meeting and that we should remove it to save some space in the |
|
Yes, we did. I just mean in terms of which branch it should go to. |
|
(and I don't think we said it was completely useless, I'm pretty sure it's been used for various cron jobs in ibis over the years, it's just not nearly as useful as its cost..) |
|
OK. I doubt this is so much of a big change though but if you prefer moving it to major, no big deal. Can do that. Just let me know. |
…rows that are useless and change the type of Value back to text
cmadjar
force-pushed
the
remove_header_row_from_parameter_file
branch
from
8ecc3e0
to
5d04e88
Compare
|
@jesscall I had to rebase the branch to major and it dismissed your review. Would you mind reviewing it again? Thanks!! |
| @@ -0,0 +1,3 @@ | |||
| DELETE FROM parameter_file WHERE ParameterTypeID=(SELECT ParameterTypeID FROM parameter_type WHERE Name='header' AND SourceFrom='parameter_file'); | |||
| DELETE FROM parameter_type WHERE Name='header' AND SourceFrom='parameter_file'; | |||
| ALTER TABLE parameter_file MODIFY Value TEXT; | |||
There was a problem hiding this comment.
@cmadjar I don't know the kind of value that are there but any value longer that 64KB will be trunct to the 1st 64KB.
If there should not be any value longer than that after the delete, I approve this PR
There was a problem hiding this comment.
@PapillonMcGill the insertion script is filtering any value longer than 1000 characters so moving it back to text is fine. The only exception was the 'header' which was crazy long and that we are getting rid of in this PR.
There was a problem hiding this comment.
Thanks for the clarification.
Brief summary of changes
Until now, the whole header of a MINC file was dumped into a row of parameter_file, taking a lot of space in the database and the SQL dump created for that table, while not being used at all.
During the imaging meeting of July 4th, a decision has been made to no longer store that row to gain some space in the database, hence, removing it from the imaging pipeline code.
This is linked to aces/Loris-MRI#453 PR on the LORIS-MRI side.
Note: the API does not look specifically for that header but dumps all the values present in the
parameter_filetable so no changes were needed.This resolves issue...
To test this change...