Various import and logging fixes #2043

ximenesuk · 2014-01-29T21:06:55Z

This PR involves several distinct parts so the review isn't very simple. For many of the features a local server is needed or beneficial. @joshmoore may have added additional test details below in the comments.

A single commit cherry-picked from editor start #1978 fixes a problem with the Editor starting up. The Editor should start with no problems.
Insight logging. A logback log file should be written to ~/omero/log/omeroinsight.log for Insight, Importer and Editor. If the config file, logback.xml, is moved from the Insight config directory the clients should still start okay but without providing any logging.
CLI in-place import options should be tested. These are detailed in https://github.com/openmicroscopy/ome-internal/pull/97
the log directory should be configurable for Python processes, see https://trac.openmicroscopy.org.uk/ome/ticket/11943
If you use -- --debug=DEBUG during import, you'll get upload status updates like 2014-01-30 ... FILE_UPLOAD_BYTES uploaded: 0 of: 0 bytes (ETA: 0' 0")

--rebased-to #2080

The individual configuration files will need to be translated via the logback tools for this to be completed.

Especially when using symlinks, there's no need to use a heavy checksum. This may not be the ideal situation, i.e. requiring the user to choose. Perhaps the `FileTransfer` API could suggest an algorithm.

Similar to `--wait` for `bin/omero chgrp` and friends, setting `--minutes-wait` for the import process disconnects once the files have been transfered. "Disconnectin from..." is printed to the CLI and the process exits. Currently, the `ImportProcessPrx` is left open on the server and must be manually closed.

Import processes returned from: ``` bin/omero shell --login client.getStatefulServices() ``` previously had very opaque (UUID-based) names. This commit adds a readable string to them.

To re-claim resources left open with `--minutes_wait`, the new `--close_completed` option checks for any process **in the current session** and closes it. ``` $ bin/omero import -- --minutes_wait=0 ... ... Disconnecting from import process... $ bin/omero import -- --close_completed Done: f318cbda-d91d-4123-bef4-467e5c4393e4/e7c27250-ac59-4337-bb0f-efd6fdb47fbd-ManagedImportProcessI 1 service(s) processed ``` For closing in other sessions, it will be necessary to first find the sessions and join them.

Entry got removed by mistake.

Java processes are being explicitly given the OMERO_LOGFILE variable. Python processes make use of omero.util.LOG_DIR for choosing a location. Now this is configurable via `omero.logging.directory`.

The node logging configuration does not make use of the `${OMERO_*}` variables. Instead, these must be set manually in the etc/*.cfg files.

ximenesuk · 2014-01-29T22:43:11Z

Closing due to a blocking problem with the IJ plugin that I neglected to test before opening.

ximenesuk · 2014-01-30T08:10:28Z

Re-opened to get into a build, note that the IJ plugin is broken in this branch.

joshmoore · 2014-01-30T08:55:36Z

--depends-on ome/bioformats#876

joshmoore · 2014-01-30T08:58:19Z

components/insight/SRC/org/openmicroscopy/shoola/env/log/LoggerImpl.java

-    	this.absFile = absFile;
-		config.put("log4j.appender.BASE.File", absFile);
-		PropertyConfigurator.configure(config);
+        LoggerContext context = (LoggerContext) org.slf4j.LoggerFactory.getILoggerFactory();


To better handle the ClassCastException from ome/bioformats#876 we might should put a try/finally around this block and handle it as if the log file were not present, i.e. disable logging if someone is choosing a non-logback implementation in imagej/fiji.

Makes sense.

pwalczysko · 2014-01-30T10:20:37Z

Editor from the artefacts on gretzky works fine (Mac OS X tested).

pwalczysko · 2014-01-30T10:39:59Z

Log gets created in the indicated place for Editor, Importer, Insight as expected. Mac OS X 10.8, artefacts from gretzky.

pwalczysko · 2014-01-30T11:24:40Z

When logback file was removed, the Insight, Editor and Importer started fine, no logs were created as expected.

After multiple imports, it's now possible to wait for all of them to complete in a single operation. A while loop is started which continues until no import processes are found.

joshmoore · 2014-01-31T09:29:42Z

@pwalczysko : the previous commit should improve the *wait* documentation of advanced-help

pwalczysko · 2014-01-31T12:24:45Z

[petr@LS25165 ~/Work/omero5-server/OMERO-CURRENT]$ bin/omero import -- --advanced-help
Previous session expired for user-4 on localhost:4064
Server: [localhost]
Username: [user-4]
Password:
Created session 8d51c9e0-aca7-4757-b22b-5d805e60503b (user-4@localhost:4064). Idle timeout: 10.0 min. Current group: read-only-1
2014-01-31 12:02:22,234 563        [      main] INFO          ome.formats.importer.ImportConfig - OMERO Version: 5.0.0-rc1-306-77e3fe7-ice33-b155
2014-01-31 12:02:22,247 576        [      main] INFO          ome.formats.importer.ImportConfig - Bioformats version: 5.0.0-rc1-306-77e3fe7-ice33-b155 revision: 8a14773 date: 31 January 2014

Advanced arguments:

  These options are not intended for general use. Make sure you have read the
  documentation regarding them. They may change in future releases.

  --transfer=ARG            File transfer method

      Values:               
       -t upload            # Default
       -t ln                # Use hard-link. Locally only!
       -t ln_rm             # Caution! Hard-link followed by rm. Locally only!
       -t ln_s              # Use symlnk. Locally only!
       -t some.class.Name   # Use a class on the CLASSPATH

  --checksum_algorithm=ARG  Choose a possibly faster algorithm for detecting file corruption
                            E.g. Adler-32 (fast), CRC-32 (fast), MD5-128
                                 Murmur3-32, Murmur3-128, SHA1-160 (slow, default)

  --minutes_wait=ARG        Choose how long the importer will wait on server-side processing
                            ARG > 0 implies the number of minutes to wait.
                            ARG = 0 exits immediately. Use a *_completed option to clean up
                            ARG < 0 waits indefinitely. This is the default

  --close_completed         Close lingering processes left by --minutes_wait

  --wait_completed          Wait for all background imports to complete.

ex. importer-cli --transfer=ln_s --checksum_algorithm=CRC-32 foo.tiff

Reads okay, except: what is the order of the commands here ? It is neither alphabetical nor according to importance.

pwalczysko · 2014-01-31T12:29:20Z

Further to the output of the -- --advanced help above: More importantly, why does it say on the soft-link locally only ! ? - The doc seems to suggest this is the only option for non-local import ?

pwalczysko · 2014-01-31T12:40:27Z

Edited my default.xml file and put in <variable name="OMERO_LOGS" value="tmp/logs/"/>
after bin/omero admin restart I have

[petr@LS25165 ~/Work/omero5-server/OMERO-CURRENT/tmp/logs]$ ls
Blitz-0.log      Indexer-0.log    PixelData-0.log  Processor-0.log  Tables-0.log

Still looks like too little of them.
But it is an improvement from 3 -> 5.

ximenesuk · 2014-01-31T12:55:29Z

The lack of logs may be because the three DropBox has failed to start for some reason. You could check the status with diagnostics.

ximenesuk · 2014-01-31T13:11:28Z

@pwalczysko Do you have the logs master.* available for the above logging test? I've used the same config as you and I'm seeing this problem in master.out

12:59:01,718 |-ERROR in ch.qos.logback.core.rolling.RollingFileAppender[FILE] - Failed to create parent directories for [/Users/cblackburn/Work/repos/ome/dist/tmp/log/Indexer-0.log]

and in master.err several lines of this form

IOError: [Errno 2] No such file or directory: '/Users/cblackburn/Work/repos/ome/dist/tmp/log/DropBox.log'

ximenesuk · 2014-01-31T13:25:48Z

I'm a little concerned with the log file location 5daac78 commit. On a fresh start I had:

jrs-macbookpro-25031:ome cblackburn$ ls dist/tmp/log/
Blitz-0.log Indexer-0.log   PixelData-0.log

I restarted and things were okay:

jrs-macbookpro-25031:ome cblackburn$ ls dist/tmp/log/
Blitz-0.log     FileServer.log      MonitorServer.log   Processor-0.log
DropBox.log     Indexer-0.log       PixelData-0.log     Tables-0.log

But the problem on the first start meant that the Python processes failed to start at all due to exceptions when trying to write to the log files. /cc @joshmoore

pwalczysko · 2014-01-31T13:26:03Z

After bin/omero admin diagnostics I have

.[petr@LS25165 ~/Work/omero5-server/OMERO-CURRENT]$ bin/omero admin diagnostics
…

Log files:  master.err                     11.0 KB       errors=6    warnings=1

pwalczysko · 2014-01-31T13:28:21Z

@ximenesuk I will send you the master.err via e-mail.
Sent it already, cc @joshmoore on the e-mail

ximenesuk · 2014-01-31T13:41:50Z

Thanks @pwalczysko looks like there is a problem creating some of log files when the server first starts and the log directory does not yet exist. This time I got

jrs-macbookpro-25031:ome cblackburn$ ls dist/tmp/log/
Blitz-0.log Indexer-0.log   PixelData-0.log Processor-0.log Tables-0.log

but restarting gave me all the logs. @joshmoore it looks like there is some sort of ordering problem but I'm not sure why that wouldn't already occur with the default var/log/

joshmoore · 2014-01-31T14:00:09Z

Re: Log files: There is certainly an issue with the initialization of the log directory, as well as its use by diagnostics. However, if someone is trying to set up the system in this way, they will almost certainly have created the log directory first, so I wouldn't be too concerned.

joshmoore · 2014-01-31T14:01:04Z

Re: advanced-help: I tried to make the order what I understood best, @pwalczysko. I'll try another version and see what you think. If you have any suggestions, let me know.

ximenesuk · 2014-01-31T14:18:46Z

Fair point @joshmoore I'm just too used having things done for me!

joshmoore · 2014-01-31T14:44:16Z

New advanced help:


ADVANCED OPTIONS:

  These options are not intended for general use. Make sure you have read the
  documentation regarding them. They may change in future releases.

  In-place imports:
  -----------------

    --transfer=ARG              File transfer method

        General options:        
         -t upload              # Default
         -t some.class.Name     # Use a class on the CLASSPATH

        Server-side options:    
         -t ln                  # Use hard-link.
         -t ln_s                # Use symlnk.
         -t ln_rm               # Caution! Hard-link followed by source deletion.

    --checksum_algorithm=ARG    Choose a possibly faster algorithm for detecting file corruption
                                E.g. Adler-32 (fast), CRC-32 (fast), MD5-128
                                     Murmur3-32, Murmur3-128, SHA1-160 (slow, default)

  Background imports:
  -------------------

    --minutes_wait=ARG          Choose how long the importer will wait on server-side processing
                                ARG > 0 implies the number of minutes to wait.
                                ARG = 0 exits immediately. Use a *_completed option to clean up
                                ARG < 0 waits indefinitely. This is the default

    --close_completed           Close completed imports

    --wait_completed            Wait for all background imports to complete.


  ex. importer-cli --transfer=ln_s --checksum_algorithm=CRC-32 foo.tiff

Report bugs to <ome-users@lists.openmicroscopy.org.uk>

Thoughts, @pwalczysko ?

pwalczysko · 2014-01-31T15:16:14Z

@joshmoore: Looks nice. But is a Background Import not an In-place import as well ? If the answer is yes, then how about to put my favourite --minutes_wait option into the example at the bottom ?

pwalczysko · 2014-01-31T15:58:22Z

Now I understand that these are independent options. What would be actually quite helpful then, is to have a second example, which would deal with --minutes_wait

joshmoore · 2014-01-31T16:09:35Z

Example added.

joshmoore · 2014-01-31T16:12:40Z

(Sorry, pushed to my branch. @ximenesuk will have to push for the PR)

ximenesuk · 2014-02-02T19:02:24Z

I'm merging @jburel's PR so that this can be tested more widely on Monday in case I have some local problems.

Removing the dependency on a Bio-Formats PR.

pwalczysko · 2014-02-03T12:37:45Z

Not sure how to test the last merge. If you want to go ahead and merge first, do it, I am happy, cos the comments start to be too long here now.

ximenesuk · 2014-02-03T15:07:38Z

Reverting to this PR before Insight merge. The Insight work to support the plug-in and changes to jars can be done as a separate PR against the main branch.

Given the testing undertaken by @pwalczysko this PR may now be good to merge!
/cc @joshmoore @jburel @melissalinkert

joshmoore · 2014-02-04T08:05:51Z

Merging. There will be several more PRs to finalize this work.

Various import and logging fixes

joshmoore and others added 17 commits January 29, 2014 16:22

Re-activate import --debug support

0cc0af8

Add time estimate to CLI output

f849535

Initial migration of insight to slf4j

08370c2

The individual configuration files will need to be translated via the logback tools for this to be completed.

Configure CLI to use etc/logback-cli.xml

8f63572

Add newline to logback-cli.xml pattern

f55a819

Add --checksum_algorithm from the CLI

132f68f

Especially when using symlinks, there's no need to use a heavy checksum. This may not be the ideal situation, i.e. requiring the user to choose. Perhaps the `FileTransfer` API could suggest an algorithm.

Improve name of servants

1e99fb6

Import processes returned from: ``` bin/omero shell --login client.getStatefulServices() ``` previously had very opaque (UUID-based) names. This commit adds a readable string to them.

Fix longopt number for --transfer

37a5555

Remove short-form -t for transfers

1de5047

Switch Insight to slf4j.

01de630

Add missing log file entry.

5389180

Entry got removed by mistake.

Fix logback pattern as x flag (NDC) is unsupported.

99fe4b1

Update some commented-out log4j with slf4j.

cd27cdb

Make LOGDIR configurable for Python processes (See #11943)

5daac78

Java processes are being explicitly given the OMERO_LOGFILE variable. Python processes make use of omero.util.LOG_DIR for choosing a location. Now this is configurable via `omero.logging.directory`.

Add pointer to master.{out,err} config (See #11943)

af8cc77

The node logging configuration does not make use of the `${OMERO_*}` variables. Instead, these must be set manually in the etc/*.cfg files.

ximenesuk mentioned this pull request Jan 29, 2014

editor start #1978

Merged

ximenesuk closed this Jan 29, 2014

ximenesuk reopened this Jan 30, 2014

joshmoore mentioned this pull request Jan 30, 2014

No log4j in loci tools ome/bioformats#876

Closed

joshmoore reviewed Jan 30, 2014
View reviewed changes

Remove unused log4j-cli.properties file.

baf3749

Add --wait_completed

64092f3

After multiple imports, it's now possible to wait for all of them to complete in a single operation. A while loop is started which continues until no import processes are found.

Updated advanced help for clarity

dc3d830

Add example of wait_completed

9c4d2de

mtbc mentioned this pull request Feb 3, 2014

fix #11877: move import logs to upload jobs instead of annotations #2055

Merged

De-personalise some todo comments.

2a9c6c4

joshmoore added a commit that referenced this pull request Feb 4, 2014

Merge pull request #2043 from ximenesuk/import-fixes

a7a987b

Various import and logging fixes

joshmoore merged commit a7a987b into ome:dev_5_0 Feb 4, 2014

ximenesuk mentioned this pull request Feb 10, 2014

Various import and logging fixes (rebased onto develop) #2080

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Various import and logging fixes #2043

Various import and logging fixes #2043

ximenesuk commented Jan 29, 2014

ximenesuk commented Jan 29, 2014

ximenesuk commented Jan 30, 2014

joshmoore commented Jan 30, 2014

joshmoore Jan 30, 2014

ximenesuk Jan 30, 2014

pwalczysko commented Jan 30, 2014

pwalczysko commented Jan 30, 2014

pwalczysko commented Jan 30, 2014

joshmoore commented Jan 31, 2014

pwalczysko commented Jan 31, 2014

pwalczysko commented Jan 31, 2014

pwalczysko commented Jan 31, 2014

ximenesuk commented Jan 31, 2014

ximenesuk commented Jan 31, 2014

ximenesuk commented Jan 31, 2014

pwalczysko commented Jan 31, 2014

pwalczysko commented Jan 31, 2014

ximenesuk commented Jan 31, 2014

joshmoore commented Jan 31, 2014

joshmoore commented Jan 31, 2014

ximenesuk commented Jan 31, 2014

joshmoore commented Jan 31, 2014

pwalczysko commented Jan 31, 2014

pwalczysko commented Jan 31, 2014

joshmoore commented Jan 31, 2014

joshmoore commented Jan 31, 2014

ximenesuk commented Feb 2, 2014

pwalczysko commented Feb 3, 2014

ximenesuk commented Feb 3, 2014

joshmoore commented Feb 4, 2014

Various import and logging fixes #2043

Various import and logging fixes #2043

Conversation

ximenesuk commented Jan 29, 2014

ximenesuk commented Jan 29, 2014

ximenesuk commented Jan 30, 2014

joshmoore commented Jan 30, 2014

joshmoore Jan 30, 2014

Choose a reason for hiding this comment

ximenesuk Jan 30, 2014

Choose a reason for hiding this comment

pwalczysko commented Jan 30, 2014

pwalczysko commented Jan 30, 2014

pwalczysko commented Jan 30, 2014

joshmoore commented Jan 31, 2014

pwalczysko commented Jan 31, 2014

pwalczysko commented Jan 31, 2014

pwalczysko commented Jan 31, 2014

ximenesuk commented Jan 31, 2014

ximenesuk commented Jan 31, 2014

ximenesuk commented Jan 31, 2014

pwalczysko commented Jan 31, 2014

pwalczysko commented Jan 31, 2014

ximenesuk commented Jan 31, 2014

joshmoore commented Jan 31, 2014

joshmoore commented Jan 31, 2014

ximenesuk commented Jan 31, 2014

joshmoore commented Jan 31, 2014

pwalczysko commented Jan 31, 2014

pwalczysko commented Jan 31, 2014

joshmoore commented Jan 31, 2014

joshmoore commented Jan 31, 2014

ximenesuk commented Feb 2, 2014

pwalczysko commented Feb 3, 2014

ximenesuk commented Feb 3, 2014

joshmoore commented Feb 4, 2014