Task #11877 (closed)
Move log files away from annotations
Reported by: | jamoore | Owned by: | mtbcarroll |
---|---|---|---|
Priority: | critical | Milestone: | 5.1.0-m1 |
Component: | Services | Version: | 5.0.0-rc1 |
Keywords: | n.a. | Cc: | java@… |
Resources: | n.a. | Referenced By: | n.a. |
References: | n.a. | Remaining Time: | 0.0d |
Sprint: | n.a. |
Description
My primary model worry about 5.0.0 not containing the refactoring(s) that will come in 5.1.0 is that the log file is an annotation rather than being stored in a proper column. If possible, it would be good to change this for release.
Note: a DB upgrade script would be required from 5.0__0
Change History (25)
comment:1 Changed 6 years ago by mtbcarroll
- Owner set to mtbcarroll
comment:2 Changed 6 years ago by jburel
comment:3 Changed 6 years ago by mtbcarroll
I'm just now starting to look at how to do it, so time is rather tight if we want to have this done this week. At a glance I'd guess that references to FileAnnotationData.LOG_FILE_NS will help to find the relevant parts of Insight, though how easily (given object hydration issues) Insight can traverse the fileset-job links to get hold of the original files attached to the upload job, I've no idea.
comment:4 Changed 6 years ago by mtbcarroll
I'll let you know when the server side is done, should be fairly quick, so that you can look at the client side while I work on an upgrade script.
comment:5 Changed 6 years ago by jamoore
If we can replace the two gateway usages (Java/Python?) in the same PR, then we should have no breakage.
comment:6 Changed 6 years ago by mtbcarroll
I'm making progress; I expect to be able to provide a first draft of the server side of this tomorrow.
comment:7 Changed 6 years ago by mtbcarroll
The server side seems to work, though perhaps it's not following an intended pattern. It can be fetched from https://github.com/mtbc/openmicroscopy/tree/trac-11877-import-log which also includes an ImportLibrary example of how to map from fileset ID to import log ID (perhaps that should actually be a function offered in the gateway). Some client-side work like Insight's ImporterUIElement.setImportLogFile remains to be done. I suggest a plan of:
- Josh ensures he isn't 5.0.0-blockingly-alarmed by the existing commits
- Jean-Marie works on further client-side changes
- I look at how to write an upgrade script to move import logs from being file annotations to being attached to the upload jobs.
comment:8 Changed 6 years ago by jamoore
Mark:
- No blocking worries.
- I do wonder though if https://github.com/mtbc/openmicroscopy/commit/c1b96d325414a0b375bb5ef8d63b1b9549fd8a80 should maintain the old method along with the new method to make the upgrade script no necessary for 5.0.0.
comment:9 Changed 6 years ago by mtbcarroll
Upgrade script now pushed to branch. Will have to be repeated in 5.0.1 for those already on 5.0.0-rc1.
(Do people ever actually access import logs in some later session after import? At least the file is still there either way.)
comment:10 Changed 6 years ago by jamoore
There's no access to the import logs (Yet). I'd assume we're adding that in 5.0.1 or similar and so then it would be more important.
comment:11 Changed 6 years ago by mtbcarroll
Will look at adding a LongIObjectListMap IMetadata.loadLogFiles(string rootType, LongList ids) like loadLogFiles("Image", [1]) -> {1: [ OriginalFile(100)]} to make remaining client changes easy.
comment:12 Changed 6 years ago by mtbcarroll
Now pushed new IMetadata.loadLogFiles method and in ImportLibrary an example of its use client-side.
I suggest that Jean-Marie completes the Insight adjustments while I add an integration test. (So far the new method's very lightly tested!)
comment:13 Changed 6 years ago by jburel
To access import-logs post import, we will need to put that in place in client. We could do that alongside the in-place import
comment:14 Changed 6 years ago by jburel
comment:15 Changed 6 years ago by mtbcarroll
Great! With this piece included, probably it is now safe for me to open a PR and get this merged into Monday morning's build?
comment:16 Changed 6 years ago by jburel
I have imported a file w/o any issue and was able to load the log file.
comment:17 Changed 6 years ago by mtbcarroll
Thank you! https://github.com/openmicroscopy/openmicroscopy/pull/2055 opened accordingly. I'll continue working on the integration test for the new metadata service method.
comment:18 Changed 6 years ago by mtbcarroll
- Status changed from new to accepted
comment:19 Changed 6 years ago by mtbcarroll
Integration test pushed. Will keep this ticket open as a reminder of the upgrade from 5.0__0.
comment:20 Changed 6 years ago by mtbcarroll
- Milestone changed from 5.0.0 to 5.0.1
Work for 5.0.0 now in above PR.
comment:21 Changed 5 years ago by mtbcarroll
Not sure how best to handle outstanding upgrade, see https://github.com/mtbc/openmicroscopy/commit/63960098501cd9b6ef2f6d29510cb17a9717cab1#commitcomment-5677707
comment:22 Changed 5 years ago by mtbcarroll
- Milestone changed from 5.0.1 to 5.1.0
I'll get it into the 5.1 upgrade script once I get my place in the breaking PR queue.
comment:23 Changed 5 years ago by jamoore
- Milestone changed from 5.1.0 to 5.1.0-m1
Perhaps let's be optimistic and say you'll find a place in the queue in the first milestone? :)
comment:24 Changed 5 years ago by mtbcarroll
- Resolution set to fixed
- Status changed from accepted to closed
comment:25 Changed 5 years ago by Josh Moore <josh@…>
- Remaining Time set to 0
(In [09cd2fb81dd1cd70653358a6d16c32b25ac25596/ome.git] on branch develop) Merge pull request #2191 from mtbc/model-changes
fix #11663, #11664, #11877: SQL DOMAIN; import logs (rebased); FS delete log trigger
Any progress on that one. This will affect insight code.