Conversation
scolapasta
left a comment
scolapasta
left a comment
There was a problem hiding this comment.
The specific comments here are small. The general thing is about whether the API class should be an ejb or not. I wrote more extensively in slack (and we are still discussing) but the TLDR is: API classes should operate similar to backing beans and NOT be ejbs. (I think)
| if (settingsService != null && settingsService.isTrueForKey(SettingsServiceBean.Key.ExcludeEmailFromExport, false) && DatasetFieldType.FieldType.EMAIL.equals(pv.getDatasetField().getDatasetFieldType().getFieldType())) { | ||
|
|
||
| } else if (datasetFieldType.isPrimitive()) { | ||
| for (DatasetFieldValue pv : sort(fld.getDatasetFieldValues(), DatasetFieldValue.DisplayOrder)) { |
There was a problem hiding this comment.
I think it would be fair to remove this and the comment. Any reason to keep it?
|
|
||
| public static String determineFileType(File file) throws IOException { | ||
| return FileUtil.determineFileType(file, file.getName()); | ||
| // Question: why do we need this utility? - as opposed to just calling the |
There was a problem hiding this comment.
good question - I vote we just remove, up to you if you want to handle in this PR though. (as it would require changing the places you call it; maybe moving the test? Though my guess is if we don't now, we never will.
|
Just a note that Odum has hit this problem in certain of its datafiles. I can provide more information about our datafiles and filemetadata if it would be helpful, but the results are identical to those described by Mr. Ferey. |
|
Hi all, what is the status of this PR? We are also having this issue and would love to be able to clean up some of our files marked as "Unknown" formats. |
|
I will close this PR in favor of #8835 that fixes the file name issue. I just checked and it is still happening in our prod., for that specific file (id 4325124). It would be nice to figure out what is happening there. I will open a new issue for just that part, and then close this PR. My gut feeling is that it has got nothing to do with the actual functionality in the redetect command. |
|
Opened #9142 to investigate the remaining mystery part (the selectively-happening weird EJB rollback). Closing this. |
cmbz
added
FY26 Sprint 14
7 participants
What this PR does / why we need it:
See the discussion on slack for what's going on in this PR.
Which issue(s) this PR closes:
Closes #7527 (maybe?)
Special notes for your reviewer:
See the discussion on slack.
Suggestions on how to test this:
There are 2 parts of the issue. The first one is trivial - the filename/extension were not being taken into account by the redetect API; this is fairly straightforward to test.
The second part, the strange EJB rollback, is completely counter-intuitive. I have no idea how to test it, seeing how I haven't been able to reproduce it outside of prod. For example, the API consistently fails in prod. with this EJB error for the following datafile:
however, it's working perfectly fine for this id on vm-5, which is also running 5.3 on a copy of the prod. db.
It doesn't happen for any file in prod. either. It was in fact a matter of considerable luck/coincidence that when the remote installation reported the error, Danny tried the API on a random file in our archive, and got the same exact error they were getting. (But at least this means that the condition is not limited to just our server).
I didn't have any other way to fix/investigate this than by deploying a modified 5.3 in prod. on a weekend. Once again, I'm not sure how to QA this.
(edit: I will see if I can find a datafile that is similarly failing on vm-5, by trial and error)
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?:
Additional documentation: