Upgrade check updates #34
Conversation
In the very near future, it would be nice to be able to perform upgrade checks from any Bio-Formats-enabled application (e.g. MATLAB). So now we do what should have been done from the beginning, and decouple the actual upgrade checking logic from the ImageJ plugin that upgrades the version of loci_tools.jar used by ImageJ.
|
Looks good, Melissa. Obviously some of the URLs are in flux, but I'm sure that's on your radar. Which code paths will trigger this? The bf command line utilities? ImageReader usage? |
|
At the moment, the only things that trigger the upgrade checks are opening a file in ImageJ using Bio-Formats, explicitly running the ImageJ update plugin, and using the little command line utility in c96950e. I'm open to suggestions on where else to call the upgrade check, keeping in mind that it must be easy to turn off. |
|
I would think all of the command-line scripts could perform the check. In fact, since the script should know where the tools jar is, it would be possible to have it automatically upgrade which is what you often must suggest to users anyway. For each of the tools, it probably makes sense to also record the class that was used. The only issue I could see would be if there are any of the tool scripts which are used in tight loops. bfopen.m is probably a similar location, but we could further make sure that invocation only takes place once per Matlab session (by setting a JVM property "bioformats_already_checked" etc.) Use of bf as a library is more difficult. The only thing I can think of to prevent multiple calls for someone who keeps, say, re-creating ImageReader or whatever is to have a file on disk that is checked against the current timestamp. |
And while we're at it, add a method that allows the caller to know whether or not the upgrade check has been performed in the JVM session. This is helpful for making sure that the upgrade check is not called many times.
Auto-upgrading can be enabled by setting the 'autoDownload' flag; it is turned off by default.
As with the "have we already done the upgrade check?" method, this just boils down to getting/setting a system property. By default, the upgrade check is still enabled.
|
Above commits should largely address your points, @joshmoore. bfopen.m and the command line tools now call the upgrade checker, and the class will always be passed to the registry (as "bioformats.class"). A system property is set once the upgrade check has been performed for the first time in a JVM session; this is easily accessible to the caller, but it is the responsibility of the caller to decide whether or not to use it. Similarly, a system property is available that can disable all upgrade checks in the current JVM session. This is primarily useful for people who have slow or no internet connections, or those who think that upgrade checks are inherently evil. By default, of course, the check is still enabled. I still haven't added a check in ImageReader, as I'm still undecided as to whether I like that idea - it's helpful to a point, but awfully invasive. |
How on earth did I manage to commit this?!
And while we're at it, fix a stupid typo in "loci.formats.UpgradeChecker".
|
Following the general idea of pull request #41, can we also convert doUpgradeCheck into a function (instead of a subfunction) to extend its usability scope? |
|
@sbesson: do you feel up to trying that? You can issue it as a PR on this PR? |
Upgrade check: split MATLAB upgrade checking function into a separate .m file
|
After refactoring, this is the current hit to the registry: i.e. the class being posted is 'UpgradeChecker'. |
...instead of always assuming that the stack element with index 1 is what we want.
|
@joshmoore Above commit ought to fix the class name being sent to the registry. It seems to work for me, but is not so well tested. |
|
Sorry, Melissa, but now it's: Would it be worth moving to just passing the value? |
...instead of trying to guess which class called UpgradeChecker.
|
Well, 'java.lang.Thread' was what it was before. In any case, added a commit now to pass in a description of the caller - should be "ImageJ" for the ImageJ plugins, "MATLAB" for bfUpgradeCheck.m, and "Bio-Formats utilities" for the command-line tools. Not tied to those names necessarily, so happy to change them if you prefer. Only downside that I can see is that it's now on the honor system as to whether a particular caller correctly sets a description (e.g. there is nothing preventing us or anyone else from setting "ImageJ" when calling from MATLAB). |
|
I'm not too terribly worried about the lack of prevention: all the other upgrade checkers are also spoofable. |
|
|
|
Doh. This no longer merges cleanly. |
Conflicts: components/bio-formats/utils/bfopen.m
|
@joshmoore Should merge cleanly now. As @sbesson noted, there was a conflict with bfopen.m, due to the refactoring that's been going on in other branches. |
|
Melissa/Sebastien ok'd: merging. |
Clients review
Handle pinhole size.
Moves the upgrade checking logic out of the loci-plugins component, so that anyone using Bio-Formats can use the upgrade check functionality.
There is a small command-line tool to demonstrate this (loci.formats.tools.UpgradeCheck), and the ImageJ plugin has been updated accordingly.
QA also correctly shows a new column ("Bio-Formats") in the statistics reports, and there are several starts recorded which suggests that the registry check is indeed working correctly.