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 10 years ago by mtbcarroll
- Owner set to mtbcarroll
comment:2 Changed 10 years ago by jburel
comment:3 Changed 10 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 10 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 10 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 10 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 10 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 10 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 10 years ago by mtbcarroll
Upgrade script now pushed to branch. Will have to be repeated in 5.0.0 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 10 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 10 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 10 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 10 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 10 years ago by jburel
comment:15 Changed 10 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 10 years ago by jburel
I have imported a file w/o any issue and was able to load the log file.
comment:17 Changed 10 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 10 years ago by mtbcarroll
- Status changed from new to accepted
comment:19 Changed 10 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 10 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 10 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 10 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 10 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 10 years ago by mtbcarroll
- Resolution set to fixed
- Status changed from accepted to closed
comment:25 Changed 10 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.